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

docs for HTTPConnection.set_tunnel are ambiguous #55657

Closed
rfk mannequin opened this issue Mar 9, 2011 · 14 comments
Closed

docs for HTTPConnection.set_tunnel are ambiguous #55657

rfk mannequin opened this issue Mar 9, 2011 · 14 comments
Assignees
Labels
docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@rfk
Copy link
Mannequin

rfk mannequin commented Mar 9, 2011

BPO 11448
Nosy @birkenfeld, @orsenthil, @ezio-melotti, @merwok, @karlcow
Files
  • set_tunnel_doc.diff: set_tunnel docs patch
  • http.client.patch
  • issue-11448-1.patch
  • issue11448_r2.patch
  • issue11448_r3.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/orsenthil'
    closed_at = <Date 2014-03-16.20:56:35.679>
    created_at = <Date 2011-03-09.02:08:48.918>
    labels = ['type-feature', 'docs']
    title = 'docs for HTTPConnection.set_tunnel are ambiguous'
    updated_at = <Date 2014-03-16.20:56:35.677>
    user = 'https://bugs.python.org/rfk'

    bugs.python.org fields:

    activity = <Date 2014-03-16.20:56:35.677>
    actor = 'python-dev'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2014-03-16.20:56:35.679>
    closer = 'python-dev'
    components = ['Documentation']
    creation = <Date 2011-03-09.02:08:48.918>
    creator = 'rfk'
    dependencies = []
    files = ['21056', '29243', '29293', '33609', '33674']
    hgrepos = []
    issue_num = 11448
    keywords = ['patch']
    message_count = 14.0
    messages = ['130396', '130401', '130413', '181665', '183017', '183271', '183358', '183359', '207665', '208748', '208752', '208753', '209033', '213758']
    nosy_count = 10.0
    nosy_names = ['georg.brandl', 'orsenthil', 'ezio.melotti', 'eric.araujo', 'rfk', 'karlcow', 'nikratio', 'docs@python', 'python-dev', 'm1kes']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue11448'
    versions = ['Python 3.3', 'Python 3.4']

    @rfk
    Copy link
    Mannequin Author

    rfk mannequin commented Mar 9, 2011

    The docs for HTTPConnection.set_tunnel(host,port) are ambiguous. They simply say "Set the host and the port for HTTP Connect Tunnelling". But should I specify the address of the server *through* which I want to tunnel, or the address of the *endpoint* of the tunnel?

    Turns out it's the latter, but I just wasted an hour "debugging" thinking it was the former :-(

    Attached is a simple doc patch to try to clarify this issue.

    @rfk rfk mannequin assigned docspython Mar 9, 2011
    @rfk rfk mannequin added the docs Documentation in the Doc dir label Mar 9, 2011
    @orsenthil
    Copy link
    Member

    I am not able to understand what you mean by 'endpoint'.

    Actually, when using tunnels people understand that they often 'tunnel through' the proxy server and here is an example code from the tests which is going to use the set_tunnel method.

    ph = urllib.request.ProxyHandler(dict(https="proxy.example.com:3128"))
    o.add_handler(ph)

    It is the proxy server and port. (Is 3128 called the endpoint of proxy?)

    Could you please provide some more information on your interpretation and the behavior you observed?

    @orsenthil orsenthil assigned orsenthil and unassigned docspython Mar 9, 2011
    @rfk
    Copy link
    Mannequin Author

    rfk mannequin commented Mar 9, 2011

    Sorry, "endpoint" is just a noun that seemed to fit for me, I've no idea if there is a standard term for this. Perhaps "origin server" if you follow the terminology from the RFC?

    By way of example, suppose I'm running a proxy on localhost:3128 and I want to tunnel through it to connect to www.example.com:443.

    From the train of thought that "I need to tunnel through localhost:3128 to connect to www.example.com:443" my instinct was to write code like this:

      c = HTTPSConnection("www.example.com",443)
      c.set_tunnel("localhost",3128)
      c.send('...etc...')

    This doesn't work; it tries to tunnel through www.example.com to get to localhost. The correct way around is:

      c = HTTPSConnection("localhost",3128)
      c.set_tunnel("www.example.com",443)
      c.send('...etc...')

    Another way to put it: the arguments to set_tunnel are the host/port to tunnel *to*, not the host/port to tunnel *through*.

    I was only able to work this out by looking through to code for urllib2. It's not clear from the docs. Then again, sounds like my doc patch didn't make things any clearer :-)

    @m1kes
    Copy link
    Mannequin

    m1kes mannequin commented Feb 8, 2013

    I thought the same as Ryan when reading the API. The best way would have been to call "set_tunnel" -> "set_proxy" and to implement the behaviour you expect on this: setting a proxy. There are some more places at this code which are not quite clear eg:

    def putheader(self, header, *values):
      """Send a request header line to the server.

    Here the methodname "putheader" is ok but the documentation is misleading: this just adds a new header-line to the buffer, it won't send it directly. But that's the problem with naming in APIs: once it's in the code you won't get it changed that fast..

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Feb 26, 2013

    This is a possible additional example for set_tunnel, modification of python3.3/html/_sources/library/http.client.txt

    Hope it helps.

    @merwok
    Copy link
    Member

    merwok commented Mar 1, 2013

    This is a possible additional example for set_tunnel,
    Thanks. In general, our tooling expects unified diffs, such as produced by hg diff.

    modification of python3.3/html/_sources/library/http.client.txt
    That’s a copy for web hosting, the real source is in Doc/library/http.client.rst (see devguide for more info).

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Mar 3, 2013

    Ah thanks Eric, I will fix that.

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Mar 3, 2013

    ok made a proper patch on the rst file with hg diff.
    See issue-11448-1.patch

    @nikratio
    Copy link
    Mannequin

    nikratio mannequin commented Jan 8, 2014

    Is there anything that needs to be done to get this patch applied?

    It would be nice if this could be committed together with the patch in bpo-7776.

    @nikratio nikratio mannequin added the type-feature A feature request or enhancement label Jan 8, 2014
    @nikratio
    Copy link
    Mannequin

    nikratio mannequin commented Jan 22, 2014

    The patches look fine to me. They are only docpatches, so no testcase is needed.

    I have rebased them on current hg tip to avoid fuzz, but otherwise left them unchanged.

    I think this is ready to be committed.

    @birkenfeld
    Copy link
    Member

    The first sentence of the second new paragraph is a bit ungrammatical, right?

    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Jan 22, 2014

    ooops right, my bad.

    s/on port 8080. We first/on port 8080, we first/

    better?

    @nikratio
    Copy link
    Mannequin

    nikratio mannequin commented Jan 24, 2014

    Apologies, I missed that. I'll be more careful in the future. I've attached an updated patch that also adds some extra Sphinx markup, but should IMO still be credited to Ryan and Karl.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 16, 2014

    New changeset 68a257ca6be6 by Benjamin Peterson in branch '3.3':
    improve set_tunnel docs (closes bpo-11448)
    http://hg.python.org/cpython/rev/68a257ca6be6

    New changeset 5cab0ada97b2 by Benjamin Peterson in branch 'default':
    merge 3.3 (bpo-11448)
    http://hg.python.org/cpython/rev/5cab0ada97b2

    @python-dev python-dev mannequin closed this as completed Mar 16, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants