Allow reusing existing event loop for server #10164
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MINOR CHANGES - updated the logic of starting the server to prevent calling asyncio.run() from a running event loop.
Collaborator
|
@DeltaGa Thanks for the PR! This looks great 🙂 We might want to add a test to |
raethlein
added
security-assessment-completed
raethlein
approved these changes
Jan 16, 2025
Collaborator
raethlein
left a comment
raethlein
left a comment
There was a problem hiding this comment.
I applied formatting and added a unit test ✅ Looks good to me now, thanks a lot for your work!
lukasmasuch
changed the title
edegp
pushed a commit
to edegp/streamlit
that referenced
this pull request
Jan 19, 2025
## MINOR CHANGES (1) - Updated the logic of starting the server to prevent calling `asyncio.run()` from a running event loop. ## Describe your changes Refactored the server startup logic to ensure compatibility with both existing and new event loops. This prevents the `RuntimeError` encountered when `asyncio.run()` is called from a running event loop on Python 3.10.16, ## GitHub Issue Link (if applicable) No GitHub issue link available. ## Testing Plan I would be grateful if someone could test the code using the existing tests. The changes have been verified through manual testing. - Explanation of why no additional tests are needed: The refactored code has been manually tested and works as expected. - Unit Tests (JS and/or Python): [@raethlein]: Added a Python unit test to ensure that `bootstrap.run` works with and without an existing asyncio eventloop. - E2E Tests: _None._ - Any manual testing needed? Manual testing was performed to ensure the server starts correctly in both new and existing event loops. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. --------- Co-authored-by: raethlein <[email protected]>
mayagbarnes
added a commit
that referenced
this pull request
Feb 20, 2025
sfc-gh-mbarnes
pushed a commit
that referenced
this pull request
Feb 20, 2025
sfc-gh-mbarnes
pushed a commit
that referenced
this pull request
Feb 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MINOR CHANGES (1)
asyncio.run()from a running event loop.Describe your changes
Refactored the server startup logic to ensure compatibility with both existing and new event loops. This prevents the
RuntimeErrorencountered whenasyncio.run()is called from a running event loop on Python 3.10.16,GitHub Issue Link (if applicable)
No GitHub issue link available.
Testing Plan
I would be grateful if someone could test the code using the existing tests. The changes have been verified through manual testing.
Explanation of why no additional tests are needed:
The refactored code has been manually tested and works as expected.
Unit Tests (JS and/or Python):
[@raethlein]: Added a Python unit test to ensure that
bootstrap.runworks with and without an existing asyncio eventloop.E2E Tests:
None.
Any manual testing needed?
Manual testing was performed to ensure the server starts correctly in both new and existing event loops.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.