Make Jenkins implement AbstractAsyncContextManager + Improve tests/README to use it #34
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
PaarthShah
commented
Aug 10, 2024
PaarthShah
commented
Aug 10, 2024
| @@ -64,7 +65,7 @@ async def close(self) -> None: | |||
| await self.session.close() | |||
|
|
|||
|
|
|||
| class Jenkins: | |||
| class Jenkins(AbstractAsyncContextManager): | |||
There was a problem hiding this comment.
Subclassing this automatically provides the default __aenter__ method (which returns self)
PaarthShah
commented
Aug 10, 2024
PaarthShah
commented
Aug 10, 2024
| @@ -184,5 +171,3 @@ async def test_make_jenkins_version(jenkins, aiohttp_mock): | |||
| ) | |||
| version = await jenkins.get_version() | |||
| assert version == JenkinsVersion(major=2, minor=346, patch=1, build=4) | |||
|
|
|||
| await jenkins.close() | |||
There was a problem hiding this comment.
This seemed like a stray call to close and tests passed without it
|
@pbelskiy Would this be good to merge along with a version bump? 👀 |
PaarthShah
force-pushed
the
async-context-manager
branch
from
962d52c
to
974caab
Compare
PaarthShah
force-pushed
the
async-context-manager
branch
from
974caab
to
ee64a8a
Compare
PaarthShah
force-pushed
the
async-context-manager
branch
from
dd71226
to
4c10e19
Compare
pbelskiy
reviewed
Aug 14, 2024
pbelskiy
approved these changes
Aug 18, 2024
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.
I was using this for a project, and I didn't like having to write
try...finally: await jenkins.close().This is the idiomatic way to setup/close resources such as this, and I'd love to see this merged/published to pypi if possible! Tests all pass locally (including with
3.73.8, the minimum supported version of python for this project).