-
Notifications
You must be signed in to change notification settings - Fork 20
Attempt to fix ci-gets-stuck in shutdown loop #1237
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
Conversation
cac8172 to
e468e9f
Compare
| while True: | ||
| # To prevent entering a shutdown-loop, we explicitly check if there is | ||
| # an eventloop running | ||
| while asyncio.get_event_loop().is_running(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this stop when there is an exception (like there is no loop)? Or is this being run in some other run_forever() or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more about preventing it to start in the first place. The proper way is probably to do this check in select in the channels repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I think it is fine, if there is no loop an exception is raised in select(), the issue is we are blindly looping on broken code forever.
2ed033f to
5e0bf82
Compare
Signed-off-by: Leandro Lucarella <[email protected]>
If the test raised an exception in the middle of the test, the `ComponentPoolStatusTracker` was not stopped, which caused the `ComponentPoolStatusTracker` to keep running and entering a restart loop when the asyncio event loop was closed. Signed-off-by: Leandro Lucarella <[email protected]>
We suspect that the CI is stuck because the forever loop is not sleeping when no exception occurs. Signed-off-by: Mathias L. Baumann <[email protected]>
We detect if the asyncio event loop is not running and log an error message and exit instead of retrying. Signed-off-by: Leandro Lucarella <[email protected]>
`run_forever()` is now more resilient to issues with no running asyncio event loop, so it is better to use that instead of our custom naïve loop. Signed-off-by: Leandro Lucarella <[email protected]>
In an attempt to find the dependency that causes the CI to get stuck (the _max test works so it must be a min dependency)