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

ops_test.fast_forward is unsafe and can fail to restore former interval #137

Open
Gu1nness opened this issue Aug 1, 2024 · 1 comment
Open

Comments

@Gu1nness
Copy link

Gu1nness commented Aug 1, 2024

The current implementation of fast_forward is unsafe if the code in the context manager raises an exception.

This code will restore the former interval only if the code inside the context manager doesn't raise an error.
If it the case, it will never restore the update interval and the model will be left "broken".
It would be better to do something like

try:
    await model.set_config({update_interval_key: fast_interval})
	yield
finally:
   await model.set_config({update_interval_key: interval_after})

so that whatever happens, the configuration is restored in the end.
I can submit a PR with implementation and tests if you want.

@addyess
Copy link
Member

addyess commented Aug 7, 2024

Sure, please do @Gu1nness and tag me in the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants