Fix ResourceWarnings on Python 3 #21
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
Add a MultipartPart.close method, which consumers can use to close files (if any) when they're finished with them. Call it on some error paths when parts are about to go out of scope, avoiding ResourceWarnings in the test suite. Also use a context manager in MultipartPart.save_as to avoid another more straightforward ResourceWarning.
|
Looks good to me. I'll need to check with a bit more time at hand, but I think this can be merged and released without any deprecation warning phase. It is unlikely that someone expects a part to be readable after an error condition. |
|
In particular I believe that, in all the cases I touched, the part in question is literally inaccessible by callers - it hasn't yet been yielded or returned. |
cjwatson
added a commit
to cjwatson/zope.publisher
that referenced
this pull request
Aug 4, 2020
This currently adds ResourceWarnings on Python 3; that will be fixed when defnull/multipart#21 lands. Fixes zopefoundation#39.
|
Hi - would it be possible to look at this fairly soon? I think my PR to convert zope.publisher to multipart (zopefoundation/zope.publisher#55) is otherwise ready to land, but it would be a bit of a shame to land it in a state that adds ResourceWarnings. |
|
Thanks for the reminder! I just merged this and released version |
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.
Add a MultipartPart.close method, which consumers can use to close files
(if any) when they're finished with them. Call it on some error paths
when parts are about to go out of scope, avoiding ResourceWarnings in
the test suite.
Also use a context manager in MultipartPart.save_as to avoid another
more straightforward ResourceWarning.