make all manager methods async #273
Merged
Conversation
This file contains 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
allows easier, more consistent override - defer initialization to `await manager` instead of immediately (avoids invoking asyncio at import time) - instantiate default manager as part of extension loading, not at import time
jacobtomlinson
approved these changes
Feb 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
consistent API makes addressing #204 easier. I ran into issues because my approach to
list_clustersrelies on making a request to the Gateway to list clusters, which should be async, instead of relying on in-memory state which may be out-of-date. I also found it simpler to avoid holding open connections to all clusters, instead only connecting when information is requested, so the Manager is essentially stateless when working with a Gateway.Gateway clusters also have a 'pending' state, which local clusters don't seem to, which is important to consider, since connecting to a pending cluster will not succeed until/unless the cluster starts.
Also moves the global
managerstate from module global to initialized at extension load time, which works better when instantiating an asyncio task, which may not connect to the right event loop.