Skip to content

Fix premature closing of the subscriber #3

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

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Dec 4, 2023

This PR fixes a premature closing of the subscriber leading to the runner being deaf.

There is no test added here as I am counting on pytroll/posttroll#57 to reveal the problem (it does locally)

@mraspaud mraspaud added the bug Something isn't working label Dec 4, 2023
@mraspaud mraspaud requested a review from adybbroe December 4, 2023 15:11
@mraspaud mraspaud self-assigned this Dec 4, 2023
@@ -85,7 +85,7 @@ def read_config(config_file):
def run_from_new_subscriber(command, subscriber_settings):
"""Run the command with files gotten from a new subscriber."""
with closing(create_subscriber_from_dict_config(subscriber_settings)) as sub:
return run_on_messages(command, sub.recv())
yield from run_on_messages(command, sub.recv())
Copy link

Choose a reason for hiding this comment

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

How can we test this? Why was this not caught before?

Copy link
Member Author

Choose a reason for hiding this comment

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

This wasn't caught before because stopping the subscriber didn't affect the behaviour of the patched subscriber. With the change in posttroll's testing function, the patched subscriber now stops "recieving" when the subscriber is stopped. This will the exhibit the problem. So the tests are actually already sufficient to avoid the problem from coming back, but they need posttrolls main.

@mraspaud mraspaud merged commit c7a4ff9 into pytroll:main Dec 5, 2023
@mraspaud mraspaud deleted the fix-premature-subscriber-close branch December 5, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants