Skip to content

Comments

Add pull method to ExclusiveRemoteTask#1027

Merged
weinbe58 merged 3 commits intomainfrom
exclusive_access_pull2
May 2, 2025
Merged

Add pull method to ExclusiveRemoteTask#1027
weinbe58 merged 3 commits intomainfrom
exclusive_access_pull2

Conversation

@jon-wurtz
Copy link
Contributor

Add a pull method to ExclusiveRemoteTask. The same as fetch except recursive after a time.sleep with a poll time of 10 seconds.

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2025

PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-05-02 17:02 UTC

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
8455 7454 88% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/bloqade/analog/task/exclusive.py 35% 🟢
TOTAL 35% 🟢

updated for commit: 5d0bc27 by action🐍

@codecov
Copy link

codecov bot commented May 2, 2025

Codecov Report

Attention: Patch coverage is 16.66667% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/bloqade/analog/task/exclusive.py 16.66% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use recursion with an unbounded depth this is very bad practice.

self._task_result_ir = self._http_handler.fetch_results(self._task_id)
else:
time.sleep(poll_interval)
self.pull(poll_interval)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do this with a loop do not use recursion for with an unknown depth of the call stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I was wondering about this. I thought it would be okay because of the slow poll time

@weinbe58 weinbe58 merged commit 887749a into main May 2, 2025
5 of 7 checks passed
@weinbe58 weinbe58 deleted the exclusive_access_pull2 branch May 2, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants