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 #45

Closed
wants to merge 47 commits into from
Closed

Python3 #45

wants to merge 47 commits into from

Conversation

pferate
Copy link
Contributor

@pferate pferate commented Jan 8, 2015

I've done some work over the past few weeks and updated the entire package to work in Python 3 (3.3 and 3.4) as well as Python 2 (2.6 and 2.7).

I focused mainly on getting all of the tests to pass. There can still be work done to clean up some parts (deprecated imports, unnecessary portions of code, etc.). I also added a Travis-CI config and have the python3 branch passing all tests.

There was one test (test_discovery: test_methods_with_reserved_names) that would not work in both versions of python because the print statement being removed. Maybe we can choose a different reserved name or have to separate tests. I wasn't sure which way you want to go, so I have it skipping it in Python 3 for now.

I also took out a questionable test (test_http: test_media_io_base_upload_from_string_io). I'm not sure if we really want to allow uploading a binary file as a string. This works in Python 2, but is fixed and not allowed in Python 3.

pferate and others added 30 commits December 24, 2014 12:44
…n3, else BytesIO. (I feel that there is a better way to do this)
…ipping this for now for Python3, until a better way can be found.
@chacken
Copy link

chacken commented Jan 16, 2015

For anyone trying to add this in a requirements file to test it out, (it took me longer than I care to admit to figure this out)..

-e git+git://github.com/pferate/google-api-python-client@python3#egg=google-api-python-client

@chacken
Copy link

chacken commented Jan 18, 2015

@pferate There's at least one issue when calling discovery.build, the content needs to be decoded. There's another bug somewhere too but I'm having some trouble determining where it's occurring. Something to do with the headers.

web_1     |   File "/code/makers/views/accounts.py", line 133, in code
web_1     |     service = discovery.build('plus', 'v1', http=h)
web_1     |   File "/usr/local/lib/python3.4/site-packages/oauth2client/util.py", line 135, in positional_wrapper
web_1     |     return wrapped(*args, **kwargs)
web_1     |   File "/usr/local/lib/python3.4/site-packages/googleapiclient/discovery.py", line 218, in build
web_1     |     resp, content = http.request('https://www.googleapis.com/discovery/v1/apis/plus/v1/rest')
web_1     |   File "/usr/local/lib/python3.4/site-packages/oauth2client/util.py", line 135, in positional_wrapper
web_1     |     return wrapped(*args, **kwargs)
web_1     |   File "/usr/local/lib/python3.4/site-packages/oauth2client/client.py", line 547, in new_request
web_1     |     redirections, connection_type)
web_1     |   File "/usr/local/lib/python3.4/site-packages/httplib2/__init__.py", line 1139, in request
web_1     |     headers = self._normalize_headers(headers)
web_1     |   File "/usr/local/lib/python3.4/site-packages/httplib2/__init__.py", line 1107, in _normalize_headers
web_1     |     return _normalize_headers(headers)
web_1     |   File "/usr/local/lib/python3.4/site-packages/httplib2/__init__.py", line 195, in _normalize_headers
web_1     |     return dict([ (key.lower(), NORMALIZE_SPACE.sub(value, ' ').strip())  for (key, value) in headers.items()])
web_1     |   File "/usr/local/lib/python3.4/site-packages/httplib2/__init__.py", line 195, in <listcomp>
web_1     |     return dict([ (key.lower(), NORMALIZE_SPACE.sub(value, ' ').strip())  for (key, value) in headers.items()])
web_1     | TypeError: sequence item 0: expected str instance, bytes found

@methane
Copy link
Contributor

methane commented Jan 18, 2015

@chacken It's known bug of oauth2client. Please use oauth2client==1.4.2.

@JelteF
Copy link

JelteF commented Feb 3, 2015

Are there any plans for merging this?

@chacken
Copy link

chacken commented Feb 18, 2015

@pferate Here's another decoding issue.. errors.py line 48

web_1     |   File "/usr/local/lib/python3.4/site-packages/googleapiclient/errors.py", line 59, in __repr__
web_1     |     self.resp.status, self.uri, self._get_reason().strip())
web_1     |   File "/usr/local/lib/python3.4/site-packages/googleapiclient/errors.py", line 48, in _get_reason
web_1     |     data = json.loads(self.content)
web_1     |   File "/usr/local/lib/python3.4/json/__init__.py", line 312, in loads
web_1     |     s.__class__.__name__))
web_1     | TypeError: the JSON object must be str, not 'bytes'

@pferate
Copy link
Contributor Author

pferate commented Feb 18, 2015

@chacken: Thanks for the note. I addressed that issue in commit 49b3bf0.

@pferate
Copy link
Contributor Author

pferate commented Feb 18, 2015

Hi @craigcitro , I wonder if you've had a chance to look through this PR. I know it's large, but it covers the entire codebase (minus the sample files). Would you want to merge this into a python3 branch to continue work?

@craigcitro
Copy link
Contributor

hi @pferate -- i haven't had a chance to look yet, but i think @nathanielmanistaatgoogle is going to be the one doing this review. i like the idea of pushing this into a python3 branch and working there, but i'll let nathaniel make the call.

@nathanielmanistaatgoogle
Copy link
Contributor

We’re excited to get this content incorporated into google-api-python-client.

There are a lot of commits in this pull request (47!). Some of them have become out-of-date (2defcf1 adds a .travis.yml), many seem mechanical (f1542f7), some of them aren’t strictly germane to Python 3 (65c4874), and some of them show early draft work and experimentation that shouldn’t be brought into the canonical history of the code (391df3c). Of course the fault for the out-of-date commits lies with us for having let this pull request sit for so long.

We’d like to integrate these improvements in a much more piecemeal fashion. How amenable would you be to breaking this up into several smaller pull requests? We believe that we’d like to see at least:

(0) One pull request containing all of the “mechanical” Python 3-related changes to the code - those fixes made by scripts or tools that solve categories of compatibility problems across the codebase (please retain the property present in the early commits of this pull request that each single commit addresses all occurrences of exactly one problem).
(1) Some number of pull requests containing Python 3-related changes to the code made by hand.
(2) Some number of pull requests containing changes to the project configuration - .travis.yml and .tox.ini and so forth.
(3) Some number of pull requests containing improvements to the code having nothing to do with Python 3. We always appreciate clean-ups!

What do you think? Is this something you’d be willing to do?

@methane
Copy link
Contributor

methane commented Mar 3, 2015

@nathanielmanistaatgoogle #25 is "mechanical" pull request.

@pferate
Copy link
Contributor Author

pferate commented Mar 3, 2015

@nathanielmanistaatgoogle: I'll see what I can do. I tried to be as atomic as possible with the commits, so that should make it easier (and the reason for so many commits).

My workflow was to update some of the more obvious items, then I used tox and travis to point me to the other problems to fix. I didn't use any other tools like 2to3 in my branch.

@nathanielmanistaatgoogle
Copy link
Contributor

@pferate: thanks much for this work. Be advised that #25 was just merged; I hope it lightens the load in this pull request.

@pferate
Copy link
Contributor Author

pferate commented Mar 3, 2015

I'll look at that and see what I can do. Thanks!

@pferate
Copy link
Contributor Author

pferate commented Mar 3, 2015

Created PR #63 for project configuration updates (2).

Expect a lot of red for 3.3 and 3.4 until I transfer over more code.

@nathanielmanistaatgoogle
Copy link
Contributor

@pferate and @methane - since you're both now actively working within hours of one another on Python 3-related changes it might be worth just checking in with each other and making sure that you're not unintentionally creating sync issues or source conflicts? I'm not aware of any problems now; I just want to make sure that you see one another's progress.

@pferate
Copy link
Contributor Author

pferate commented Mar 4, 2015

See my latest PR #64.

@nathanielmanistaatgoogle
Copy link
Contributor

This pull request smells obviated; please close if you agree.

@pferate pferate closed this Mar 10, 2015
@JelteF JelteF mentioned this pull request Mar 10, 2015
akrherz pushed a commit to akrherz/google-api-python-client that referenced this pull request Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants