Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test client response.close() closes input stream #2041

Merged
merged 1 commit into from Feb 11, 2021
Merged

Conversation

davidism
Copy link
Member

When passing a large amount of data (> 500 KiB) with the test client, it uses a temporary file to store the encoded multipart form data. The test client has no way to close the file, and if it's left open after a test finishes Python may show a ResourceWarning. It's not entirely clear when Python decides to do this, pytest doesn't show it but unittest does, but neither show it with the latest Werkzeug code unless -Walways is used.

Now, the input stream is closed automatically when following redirects (except for 307 and 308, which preserve the body), and the final request's stream.close method is added to the response's close callbacks. So calling response.close() will close the input stream in addition to the response stream.

I couldn't figure out a reliable way to make a test for this, given how inconsistent the warning is in the first place, but all current tests pass.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism davidism added this to the 2.0.0 milestone Feb 11, 2021
@davidism
Copy link
Member Author

davidism commented Feb 11, 2021

And in the first interesting win for type annotations, mypy caught that environ_property for request.input_stream was being used incorrectly, treating the doc string as the default value.

@davidism davidism merged commit 40dc95b into master Feb 11, 2021
@davidism davidism deleted the client-close-input branch February 11, 2021 22:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sending >=500 KB file with test client causes warning
1 participant