Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

status: do not create thread to run callbacks if timeout is None #1182

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

mguijarr
Copy link
Collaborator

@mguijarr mguijarr commented Mar 19, 2024

Here is a merge request that mitigates concern expressed here: #837 (comment) (also linked with issue #844).

I have the impression, a simple optimization is to not create a thread if it is not needed (in particular when timeout is set to None, which seems to be most of the time).

Otherwise some background about the problem Status objects want to solve with the callbacks thread would be insightful.

@mguijarr mguijarr force-pushed the status_callback_thread branch 2 times, most recently from e7651f9 to 3cd574a Compare March 19, 2024 18:21
@tacaswell
Copy link
Contributor

Thank you for this, this seems sensible to me and looks correct to me.

Can you add a test in test_status.py to cover the case with no timeout but a settle time?

@mguijarr mguijarr force-pushed the status_callback_thread branch from 3cd574a to 61745f3 Compare March 21, 2024 08:27
@mguijarr
Copy link
Collaborator Author

@tacaswell Done

@mguijarr mguijarr force-pushed the status_callback_thread branch from 61745f3 to 2778262 Compare March 21, 2024 08:31
@tacaswell tacaswell merged commit 2722e37 into bluesky:master Mar 21, 2024
7 of 8 checks passed
@tacaswell
Copy link
Contributor

Thank you @mguijarr !

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