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

Properly close 'file://' resources #1393

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CharString
Copy link

I bumped into this when opening several wsdl which in turn opened lots
of xsd, with 'file://' scheme.

The issue was that the resp.raw.close nor resp.raw.release_conn set
in the FileAdapter were ever called.

It's unclear to me whether this should be fixed in requests. It doesn't
do that great a job at resource management for the naive user aka
Human™. It makes sense to me that exhaustively reading Response.raw
should close it unless the caller explicitly set stream on the
request. Probably by using this closing pattern in the generator in
Response.iter_content.

Workarounds without this fix:

  • using a scheme-less url as zeep will assume it's a local path and
    open the file as a context manager.
  • use one of the caches from zeep.cache to hide duplicate open resources.

I bumped into this when opening several wsdl which in turn opened lots
of xsd, with 'file://' scheme.

The issue was that the `resp.raw.close` nor `resp.raw.release_conn` set
in the `FileAdapter` were ever called.

It's unclear to me whether this should be fixed in requests. It doesn't
do that great a job at resource management for the naive user aka
Human™. It makes sense to me that exhaustively reading `Response.raw`
should close it unless the caller explicitly set `stream` on the
request. Probably by using this `closing` pattern in the generator in
`Response.iter_content`.

Workarounds without this fix:
 - using a scheme-less url as zeep will assume it's a local path and
   open the file as a context manager.
 - use one of the caches from `zeep.cache` to hide duplicate open resources.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant