Skip to content

Fix non-returned value#12

Open
pbitzer wants to merge 1 commit into
masterfrom
state_monitor_patch
Open

Fix non-returned value#12
pbitzer wants to merge 1 commit into
masterfrom
state_monitor_patch

Conversation

@pbitzer

@pbitzer pbitzer commented Oct 21, 2021

Copy link
Copy Markdown
Contributor

If a state variable was not present, or had a bad/NaN value, an exception would be raised. In this case, the _previous_data attribute would not be updated, and the error would persist at future steps. This makes sure previous data is update AND make sure the input data is returned for other steps in the brokkr chain.

If a state variable was not present, or had a bad/NaN value, an exception would be raised. In this case, the `_previous_data` attribute would not be updated, and the error would persist at future steps. This makes sure previous data is update AND make sure the input data is returned for other steps in the brokkr chain.
@pbitzer
pbitzer requested a review from CAM-Gerlach October 21, 2021 02:22
@CAM-Gerlach CAM-Gerlach added bug Something isn't working and removed bug Something isn't working labels Oct 21, 2021

@CAM-Gerlach CAM-Gerlach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, since the try block this finally block attaches to already has an except Exception that doesn't terminate execution (and no else block), in case of an error caught by it, execution just continues to these two lines anyway if any error occurs without this change. The only scenario this makes any difference in is if a KeyboardInterrupt or similar BaseException occurs during execution, in which case you want to terminate immediately and which is already caught and handled anyway by the custom signal/interrupt handler. So I'm not sure this PR really makes any difference?

@CAM-Gerlach

Copy link
Copy Markdown
Member

@pbitzer should we close this?

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