release_conn parameter doesn't have effect (without preload_content) about urllib3 HOT 5 CLOSED

@shazow In the comment to release_conn parameter to urlopen method you wrote

This is useful if you're not preloading the response's content immediately.
It suggests we can't return connection to the pool before loading response's content but that's not the case, is it?

I guess the idea was to either release connection in HTTPConnectionPool.urlopen() (if it's to be released automatically) or to pass it to HTTPResponse (if it's not to be released automatically). Automatically releasing connection in HTTPResponse.read() goes against this idea by releasing connection which should not be released (that's why it was passed to HTTPResponse). Removing code that releases connection in HTTPResponse.read() makes all tests pass. What do you think?

from urllib3.

Alright I reinvestigated what I did when I wrote this and here's what's going on:

release_conn only has an effect when preload_content=False.

As you noticed, there's a convenience feature which automagically releases the connection once the full response has been read even when you specify release_conn=False. To prevent this, you need to also specify preload_content=False.

The reason for this is because we wanted to make preload_content=False be default eventually (to promote on-demand loading which would make us perform better in benchmarks and generally more sane behaviour) and having release_conn behave this way was a good way to set expectations for a future release without breaking backwards compatibility.

So here are the options:

1. Remove this convenience feature and risk people forgetting to manually release the connection even after they've read the entire response. (Can we think of scenarios when we'd want to not release the connection even after we've read the entire response?)
2. Document release_conn param more thoroughly to describe the dependency on preload_content.
3. Make preload_content=False default as planned in the next release.

I vote for 2 or 3.

from urllib3.

I'm not sure if I get this right.
The paramount question is this: is releasing a connection orthogonal to reading response's body? If it is I see no reason to make release_conn depend on preload_content.

Can we think of scenarios when we'd want to not release the connection even after we've read the entire response?

Yes, I think we should allow this. Someone, for instance, may want to do some request manually using such loose connection and then eventually return such connection to the pool.
The opposite situation is what I asked about here

It suggests we can't return connection to the pool before loading response's content but that's not the case, is it?

How about having 3 different states for release_conn: immediately, on_response_read and never?

from urllib3.

Oops, that last commit should have been issue 23, not 22. Disregard it.

from urllib3.

Looks like we've moved on from this.

from urllib3.