charmed-kubernetes / pytest-operator

Apache License 2.0
7 stars 13 forks source link

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

Open Gu1nness opened 3 months ago

Gu1nness commented 3 months ago

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 commented 3 months ago

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