Support for specifying a source address for connections about urllib3 HOT 35 CLOSED

Very cool!

The goal for urllib3 at this time is to be completely backwards compatible to py25. Can you think of a way to do this with source address?

For now, you could extend your own HTTP(S)ConnectionPool class and override the _new_conn method to include a source address. Not an ideal solution, I agree.

I'll keep this issue in mind for future refactoring, in case I can make it easier, but I don't expect we'll make it a first-class feature unless there's a backwards compatible way to do it or the world abandons 2.x altogether and we start focusing on 3.x. :P

from urllib3.

How about supporting this only in Python 2.7 or greater? This is much better than not supporting it at all :)

from urllib3.

I disagree — compatibility with 2.6 and 2.6 is extremely important.

from urllib3.

Backports are often feasible.

from urllib3.

This would be optional feature. If someone wants to be compatible with Python <2.7 it's enough to not use it.

from urllib3.

Ah, gotcha. I thought you meant dropping 2.5–2.6 support entirely.

from urllib3.

Let's call it progressive enhancement in Python :)

from urllib3.

Hmm. I'm not opposed to starting a branch with py27-only features but I imagine it's going to be annoying and confusing to have py27-only features in the mainline release in terms of documentation and support.

Perhaps it's a good first step though, start ironing things out and then merging it in if it becomes compelling enough or if we find a reasonable backport solution.

from urllib3.

I don't see any problems with having it in the mainline. As to documentation; the only thing is to add memo/warning such feature is available only in 2.7.

from urllib3.

1. Users don't read documentation. :) We'll inevitably get "why doesn't this work?"
2. It's a slippery slope. Once we open the floodgate of depending on non-py25 supported features, it's likely the code will get increasingly dependent on it.
3. Spaghetti code. We'll need to have specific branches of code just for py27. I'm dreading how we'd go about porting to py3.

Hmm how about this:

Since we'd already encounter all of these issues when porting to py3, how about we target this feature for the py3 version of urllib3?

from urllib3.

ad. 1.
To be on the safe side we could just throw with clear error message if some feature is used with unsupported Python version.

ad. 2
Not necessarily. Taking this request as an example; giving source address is isolated in its nature and doesn't influence the rest of code.

ad. 3
Yes, we would have specific branches of code. However, the problem of spaghetti code is universal. If we know how to avoid it in other areas we can use the same techniques to avoid it in this situation.

As to py3; that's something worth to consider.

from urllib3.

kennethreitz/requests#394

from urllib3.

So this is a show stopper for me over in requests (I'm working against an IP authenticated API) such that I may be able to justify fixing this for real. I've spent a lifetime total of 30 mins thinking about this problem / looking at the urllib3 code base so I'm very open to suggestions...

What would you want the API to look like?

Regardless of what/how the underlying 2.7 functionality addressed in the library this change is going to require an urllib API innovation as the stdlib urllib doesn't actually expose this. Adding source_address onto RequestMethods.request seems like the obvious answer to me.

What would you like vs. never merge from an architectural standpoint? What would you want version support to look like?

The classes of solution I can imagine are:

1. Back port the stdlib functionality at run time or "pip time"

• monkey patch of the standard library to "back port at run time" support for source address into socket and httplib then use the stdlib in urllib3 without regard for version.
• shipping a "custom" (ie lifted from 2.7) socket and [httplib] with urllib3

2. Introduce version dependent features and either:

• version checking to ask permission, or
• error catching / exception passing to beg forgiveness

None of these seem great but based on the previous discussion here it looks like version dependent enhancement and begging forgiveness on <2.7 is the preferred solution. Is that the case? If so is their an "obvious" single point of interaction between urllib3 and the standard library? It looks like VerifiedHTTPSConnection and HTTPConnectionPool use the standard lib independent but those may be the only places that would have to have custom error handling around passing a source address.

from urllib3.

In https://github.com/kennethreitz/requests/issues/995 sounds like people are happy with a NotImplementedError when source_address is specified on Py26. Are you opposed to that?

I rather avoid backporting such a significant module as socket just for a single version.

from urllib3.

It only makes sense in all candor.

from urllib3.

I have a branch with working source_address support in Requests and Urllib3 ( http://github.com/jessesherlock/requests ), it's in a rough state at the moment but it's working nicely for me in 2.7. Some feedback would be appreciated.

from urllib3.

@jessesherlock Would you like to make that into a PR so we can review the diff?

from urllib3.

@shazow No problem, my bad for assuming you could view diffs on forks in github without one.

https://github.com/kennethreitz/requests/pull/1288

from urllib3.

This feature is really needed.
It would be great if we could use interface name directly too without specifying IP address. (e.g. ppp0, wlan0). With this we wouldn't need to write wrappers. It is needed in case interface changes IP address at runtime.

from urllib3.

Anyone want to do a PR for this? Ideally needs to be backwards compatible with Py26.

@takdragon, @chocis, @jessesherlock?

from urllib3.

My mess of a branch needs to be redone using connection adapters, which I kinda knew from the beginning but I needed the functionality immediately and didn't have a good understanding of how connection adapters worked in requests.

My branch may be useful for reference but the approach is wrong from a high level, so it's not within re-factoring distance of a proper PR.

My current workload and schedule means I won't have time to do this but I will have time to help out if anyone does take it on.

from urllib3.

@freshhawk Keep in mind, Connection Adapters is a Requests thing, not a urllib3 thing. :) Getting source_address into urllib3 should be a smaller subset of work.

from urllib3.

Heh, should have scrolled higher up before responding, I thought this was one of the many related Requests threads!

That amount of work is more doable for me, although my realistic timeline is measured in weeks as my startup is in a fundraising/crunch mode right now

Is everyone settled on throwing NotImplementedError when source_address is used on < 2.7?

from urllib3.

@freshhawk Ooo best of luck! I know that feel. :)

Yes I think that'd be a good start.

from urllib3.

Do we have a timeline to release this enhancement?

from urllib3.

@takdragon Whenever someone writes a PR for it. Wanna take a crack at it?

from urllib3.

I have at least another 2 weeks before I can try to wrap this up due to day job commitments (or over-commitments really). If anyone else wants to take a shot please do.

from urllib3.

Anyone working on this?

from urllib3.

Actually this should be at least partly supported now, I think. You can now pass source_address to new connections but it will be ignored in older Pythons.

I am not sure there is a way to do this from the ConnectionPool-layer though. It would be swell if somebody could document what we have, though. :)

@bui Could you share what your use case is? We can try to figure out what changes need to be made to support it.

If you'd like to get involved with this, I'd very much appreciate it.

from urllib3.

I'm close to getting it to work, though would it be best to set the source_address when creating the PoolManager, or as an argument when making a .request()?

from urllib3.

@bui That's a good question.

I suspect adding **connection_kw to the HTTP*ConnectionPool constructor which would get propagated through PoolManager(..., **connection_pool_kw) might be the way to go.

Since source_address is per-HTTPConnection rather than per-request, I'd say it doesn't make sense to have it as an argument of .request() (and thereby .urlopen()).

I look forward to your PR! :)

from urllib3.

@shazow https://github.com/bui/urllib3/commit/d00556a6315c8f437c4d55f87ecb515d16f7338a

This makes it work for HTTP connections, but not for HTTPS. It's something to do with HTTPSConnectionPool, but I can't figure it out. Any ideas?

(I already tried adding source_address to the conn = self.ConnectionCls(..) after that commit but it didn't do anything)

from urllib3.

@bui I left a comment but I'm not sure. What do you mean it doesn't work for HTTPS? What's the error?

Please make a PR so we can move the discussion there. :)

Also I'd prefer to avoid adding an explicit source_address param. I like the idea of a **connection_kw which we can merge self.strict into.

from urllib3.

@kennethreitz Since new code of urllib3 has supported the source_address parameter for Python version 2.7, may something be done for requests to support it? This feature is really useful and needed. Even just in version 2.7.

from urllib3.

@bofortitude Probably better to open this on the Requests project.

from urllib3.