Make Client support context manager protocol #104
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
Clients use thread-based executors under the hood, but we did not provide any mechanism to shut them down. This is bad design as it means there's no reliable way to limit the number of spawned threads if you need to create many clients (e.g. while running a test suite where new clients are being created in each test). Make it work in a 'with' statement so it's possible to shut down all threads associated with the client; attempting to use a client after shutdown will raise an error. It's not mandatory, but usage of 'with' is recommended, so several examples and docs were updated to do that. FakeClient doesn't use any threads but is intended to have as close as possible behavior to a real Client, so it also raises the same errors if used after shutdown. This is similar in spirit to release-engineering/pushsource@1ebda2c.
rbikar
approved these changes
Jul 23, 2021
negillett
approved these changes
Jul 23, 2021
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.
Clients use thread-based executors under the hood, but we did not
provide any mechanism to shut them down. This is bad design as it means
there's no reliable way to limit the number of spawned threads if you
need to create many clients (e.g. while running a test suite where
new clients are being created in each test).
Make it work in a 'with' statement so it's possible to shut down all
threads associated with the client; attempting to use a client after
shutdown will raise an error. It's not mandatory, but usage of 'with'
is recommended, so several examples and docs were updated to do that.
FakeClient doesn't use any threads but is intended to have as close as
possible behavior to a real Client, so it also raises the same errors
if used after shutdown.
This is similar in spirit to
release-engineering/pushsource@1ebda2c.