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

Close sessions created in the functional API #2326

Merged
merged 1 commit into from Nov 12, 2014

Conversation

sigmavirus24
Copy link
Contributor

This is related to #1882 and #1685. By calling close on the session, we
clear the PoolManager operated by the Session and close all sockets.

Fixes #1882
Partially-fixes #1685

This is related to psf#1882 and psf#1685. By calling close on the session, we
clear the PoolManager operated by the Session and close all sockets.

Fixes psf#1882
Partially-fixes psf#1685
@Lukasa
Copy link
Member

Lukasa commented Nov 7, 2014

Eh, I'm +0.5. I still think that #1882 is unnecessary whining on the part of CPython, but never mind. ;)

@sigmavirus24
Copy link
Contributor Author

@Lukasa it may be CPython being overly cautious but it will be a problem on PyPy. Stuff like this can cause an application to crash because there are too many file descriptors open. Others can explain why this is important to PyPy better than I can, but I'm very confident this is a worth while change.

My only qualm is worrying about users who may be accessing r.raw._pool. Granted they shouldn't be, but I'm worried about an influx of users who will complain that we broke X.

@Lukasa
Copy link
Member

Lukasa commented Nov 7, 2014

Yeah, that's a point. Can't really merge until 3.0.

@sigmavirus24
Copy link
Contributor Author

I wouldn't think users relying on a private attribute would be given priority over a bug that could so easily affect PyPy users. If they're accessing _pool I'd be:

  1. surprised and confused
  2. unforgiving because they're accessing a private attribute

I can't think of a reason anyone would need to access a urllib3 connection pool frankly other than for the horrible debugging I've been going through and ideally no one else is really doing that.

@kevinburke
Copy link
Contributor

Ship it :)

Kevin Burke
phone: 925.271.7005 | twentymilliseconds.com

On Fri, Nov 7, 2014 at 11:53 AM, Ian Cordasco notifications@github.com
wrote:

I wouldn't think users relying on a private attribute would be given
priority over a bug that could so easily affect PyPy users. If they're
accessing _pool I'd be:

  1. surprised and confused
  2. unforgiving because they're accessing a private attribute

I can't think of a reason anyone would need to access a urllib3
connection pool frankly other than for the horrible debugging I've been
going through and ideally no one else is really doing that.


Reply to this email directly or view it on GitHub
https://github.com/kennethreitz/requests/pull/2326#issuecomment-62202216
.

@stas stas mentioned this pull request Nov 8, 2014
kennethreitz added a commit that referenced this pull request Nov 12, 2014
Close sessions created in the functional API
@kennethreitz kennethreitz merged commit e4ddca0 into psf:master Nov 12, 2014
@alex
Copy link
Member

alex commented Nov 12, 2014

woo

@sigmavirus24 sigmavirus24 deleted the close-functional-sessions branch November 12, 2014 18:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ResourceWarning in python 3.2+ Possible Memory Leak
5 participants