Skip to content

Conversation

e00E
Copy link
Contributor

@e00E e00E commented Oct 10, 2015

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 10, 2015
@nathanielmanistaatgoogle
Copy link
Contributor

We dropped support for Python 2.5 and earlier a long time ago, and even Python 2.6 is ancient history these days (http://www.curiousefficiency.org/posts/2015/04/stop-supporting-python26.html). Why not just drop the version check entirely?

@e00E
Copy link
Contributor Author

e00E commented Oct 10, 2015

Complementary bug: #142

I dont know which versions you want to offically support but I do think it is reasonable to drop support for python < 2.6.

@nathanielmanistaatgoogle
Copy link
Contributor

Yeah. Let's just drop the version check entirely.

@e00E
Copy link
Contributor Author

e00E commented Oct 10, 2015

Hm it says Travis build failed, but the link only shows There was an error while loading data, please try again. I didnt test the last commit because I could not immediately figure out how to run the tests in the repository.

@nathanielmanistaatgoogle
Copy link
Contributor

When I look at https://travis-ci.org/google/google-api-python-client/jobs/84685349 I see

====================================================================== ERROR: test_resumable_media_handle_uploads_of_unknown_size (tests.test_discovery.Discovery) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/travis/build/google/google-api-python-client/tests/test_discovery.py", line 1066, in test_resumable_media_handle_uploads_of_unknown_size status, body = request.next_chunk(http=http) File "/home/travis/build/google/google-api-python-client/.tox/py27/lib/python2.7/site-packages/oauth2client/util.py", line 142, in positional_wrapper return wrapped(*args, **kwargs) File "/home/travis/build/google/google-api-python-client/googleapiclient/http.py", line 830, in next_chunk data = self.resumable.stream() File "/home/travis/build/google/google-api-python-client/googleapiclient/http.py", line 208, in stream raise NotImplementedError() NotImplementedError: -------------------- >> begin captured logging << -------------------- googleapiclient.discovery: INFO: URL being requested: POST https://www.googleapis.com/upload/zoo/v1/animals?alt=json&uploadType=resumable --------------------- >> end captured logging << --------------------- 

. Could that be a consequence of this change?

@e00E
Copy link
Contributor Author

e00E commented Oct 11, 2015

Yes it was my fault. I should have just deleted the python version checks and kept everything else intact but I thought the whole if-else block was obsolete.

@e00E
Copy link
Contributor Author

e00E commented Oct 11, 2015

Unless there is a test running on python < 2.6 im not sure why it would fail. The diff to master is now very simple again, but we cant go through the "you tell me the error message and I fix it" cycle every time as the Travis links are still broken for me.

@e00E
Copy link
Contributor Author

e00E commented Oct 11, 2015

OH this is related to my browser somehow. I just tried a different one and the page works.

@e00E
Copy link
Contributor Author

e00E commented Oct 11, 2015

This is an error in the test with the new philosophy of not supporting python < 2.6 . The test explicitly tests python 2.5.5. I think the test should be removed.

@nathanielmanistaatgoogle
Copy link
Contributor

Change content now looks great. Please squash commits (so that this pull request just has one commit) and adhere to http://chris.beams.io/posts/git-commit/#seven-rules with your commit message.

Thanks for this improvement!

Removed support for Python versions < 2.6 and fixed http.next_chunk not using streams in Python 3. Addtionally a now unneeded test was removed. Support for these old Python versions was determined unnecessary.
@e00E
Copy link
Contributor Author

e00E commented Oct 12, 2015

Thank you too for reviewing

@nathanielmanistaatgoogle
Copy link
Contributor

Passes my review. @tmatsuo do you want to be a second pair of eyes on this?

@tmatsuo
Copy link
Contributor

tmatsuo commented Oct 12, 2015

LGTM

nathanielmanistaatgoogle added a commit that referenced this pull request Oct 12, 2015
Fix file stream recognition for Python 3.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 7f85278 into googleapis:master Oct 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

4 participants