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

Python3 v0.11.0 HTTPConnectionWithTimeout AttributeError proxy_info #97

Closed
wants to merge 2 commits into from

Conversation

germanjoey
Copy link

When proxy_info=None, HTTPConnectionWithTimeout's constructor will not set proxy_info to anything, which therefore triggers an AttributeError when connect is called.

For example, this test script:

import httplib2
connection = httplib2.HTTPConnectionWithTimeout('www.example.com', 80)
connection.request('GET', '/')
connection.close()

Results in:

(venv) C02VP0WSHTD8:httplib2_test2 jryan$ python3 test.py
  Traceback (most recent call last):
    File "test.py", line 4, in <module>
      connection.request('GET', '/')
    File "/usr/local/Cellar/python/3.6.4_4/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1239, in request
      self._send_request(method, url, body, headers, encode_chunked)
    File "/usr/local/Cellar/python/3.6.4_4/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1285, in _send_request
      self.endheaders(body, encode_chunked=encode_chunked)
    File "/usr/local/Cellar/python/3.6.4_4/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1234, in endheaders
      self._send_output(message_body, encode_chunked=encode_chunked)
    File "/usr/local/Cellar/python/3.6.4_4/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1026, in _send_output
      self.send(msg)
    File "/usr/local/Cellar/python/3.6.4_4/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 964, in send
      self.connect()
    File "/Users/jryan/Documents/httplib2_test2/httplib2/python3/httplib2/__init__.py", line 897, in connect
      if self.proxy_info and socks is None:
  AttributeError: 'HTTPConnectionWithTimeout' object has no attribute 'proxy_info'

@codecov
Copy link

codecov bot commented Mar 21, 2018

Codecov Report

Merging #97 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   68.52%   68.58%   +0.05%     
==========================================
  Files           6        6              
  Lines        2456     2454       -2     
==========================================
  Hits         1683     1683              
+ Misses        773      771       -2
Impacted Files Coverage Δ
python3/httplib2/__init__.py 82.11% <100%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dee729...c95d0a6. Read the comment docs.

@temoto
Copy link
Member

temoto commented Mar 22, 2018

How come you use HTTPConnectionWithTimeout and not httplib2.Http?

@temoto
Copy link
Member

temoto commented Mar 22, 2018

@germanjoey in case you want to send another patch, here's an update on what's going on with tests.

repo/python{2,3}/httplib2test.py is obsolete and will be removed soon as we merge py2/3 code.
New test suite lives in repo/tests/ and to run it, you do script/test.

  • it passes flags to pytest, for example, script/test -svk some_test_name to show stdout, verbose and run subset of tests matching some_test_name.
  • script/test runs tests using 2.7, 3.6 and then builds a package (to verify that setup.py is not broken).
  • you may not have 2.7 or 3.6, I guess easiest way to work around that is mkdir -p ./venv-27/bin ; ln -s /bin/true ./venv-27/bin/pytest.

@temoto temoto changed the title Fix Python3 bug for httplib2 v11 for HTTPConnectionWithTimeout Fix Python3 bug for httplib2 v0.11.0 for HTTPConnectionWithTimeout Mar 22, 2018
@temoto temoto changed the title Fix Python3 bug for httplib2 v0.11.0 for HTTPConnectionWithTimeout Python3 v0.11.0 HTTPConnectionWithTimeout AttributeError proxy_info Mar 22, 2018
@temoto
Copy link
Member

temoto commented Mar 22, 2018

@germanjoey put this patch into your pull request if you want to keep authorship: cdb5d03 it also fixes same issue with HTTP-S-connection and puts test into new suite.

@germanjoey
Copy link
Author

Oh I see, thank you man! Applied your patch and pushed it again.

@sfontana
Copy link

Please merge this asap :)

@temoto
Copy link
Member

temoto commented Mar 22, 2018

Please, use httplib2.Http().request API. HTTP(S)Connection types may go away in future releases. It never was intended for external usage.

@sfontana
Copy link

@temoto aren't HTTP(S)Connection types selected automatically here https://github.com/httplib2/httplib2/blob/master/python3/httplib2/__init__.py#L1378 when connection_type is not specified?

@temoto
Copy link
Member

temoto commented Mar 22, 2018

@sfontana yes they are selected, I don't see where you're going with that.

Let me make things crystal clear:

  • yes, HTTP(S)ConnectionWithTimeout looks like public API (no underscore, etc)
  • it was only meant to be used internally by httplib2
  • there is bug with proxy_info attribute
  • this bug is not triggered by using public API httplib2.Http()
  • therefore, this is a minor issue, hurry is better applied to transition to httplib2.Http()
  • of course, no evil slowing of process is not present either, we need review to merge, then me or other maintainer can release 0.11.1 with this fix

@robin900
Copy link

@temoto httplib2 users don't necessarily have control over the code that calls HTTPConnectionWithTimeout or HTTPSConnectionWithTimeout instead of Http. In my case, that code is in oauth2client package which is itself a dependency of a package used in my project. Often httplib2 is a project's indirect dependency, with three or more levels of indirect use.

@stinky2nine
Copy link
Contributor

LGTM

@temoto
Copy link
Member

temoto commented Mar 26, 2018

Merged 5468fe3

Thank you @germanjoey

@temoto temoto closed this Mar 26, 2018
@temoto
Copy link
Member

temoto commented Mar 26, 2018

Fix was released on PyPI 0.11.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants