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

Allow using oauth2client versions < 2 #201

Merged
merged 1 commit into from
Mar 23, 2016
Merged

Allow using oauth2client versions < 2 #201

merged 1 commit into from
Mar 23, 2016

Conversation

toabctl
Copy link
Contributor

@toabctl toabctl commented Mar 14, 2016

Be compatible with older oauth2client versions. This simplifies
the installation (also for downstream) when software needs an older
oauth2client version but also wants to use googleapiclient.

Be compatible with older oauth2client versions. This simplifies
the installation (also for downstream) when software needs an older
oauth2client version but also wants to use googleapiclient.
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Mar 14, 2016
@theacodes
Copy link
Contributor

Are you sure this is the only change needed?

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Mar 14, 2016
@theacodes
Copy link
Contributor

@toabctl I'm also curious about what cases you're running into where you need a lower version of oauth2client.

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

Are you sure this is the only change needed?

No. But looking at the changelog doesn't indicate any other needed changes and also your testsuite passes :)

@theacodes
Copy link
Contributor

your testsuite passes :)

That gives me a lot of confidence.

@nathanielmanistaatgoogle any opposition to this change?

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

@toabctl I'm also curious about what cases you're running into where you need a lower version of oauth2client.

@jonparrott google-api-python-client needs oauth2client >= 2 but the google cloud sdk (https://cloud.google.com/sdk/ ) still uses oauth2client 1.X . I know it's bundled in google-cloud-sdk but distributions (I'm working for SUSE, but same for Debian, ...) bundled software is usually split so if there's a (security) fix you don't have to patch the software (in this case oauth2client) 2x. So if I want to package (RPM) and distribute google-api-python-client together with google-cloud-sdk, that's currently not possible.

@theacodes
Copy link
Contributor

@toabctl interesting, we seem to get a lot of stuff around OS-level packages. Why not use a virtualenv?

/cc @dhermes

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

And I havn't checked any other software yet. There might be more which depends on oauth2client < 2

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

@toabctl interesting, we seem to get a lot of stuff around OS-level packages. Why not use a virtualenv?

Because as a user I want to have a single package manager to handle all my files. virtualenvs are nice to play and develop but not so nice for production. If I use a couple of different languages on my system, should I use 10 different package managers to handle all my files (gem, pip, npm, ...)?

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

So if you merge this it would be really cool to get a new release on pypi btw.

@theacodes
Copy link
Contributor

virtualenvs are nice to play and develop but not so nice for production. [...]

I could definitely disagree with this sentiment, but this is neither the time nor place.

Will wait for @nathanielmanistaatgoogle's opinion on this change before merging.

@toabctl
Copy link
Contributor Author

toabctl commented Mar 14, 2016

@jonparrott but even with a virtualenv it can happen that you need to combine 2 python modules where module 1 needs oauth2client < 2 and module 2 needs oauth2client >=2

@nathanielmanistaatgoogle
Copy link
Contributor

This is weird and strange, so the barrier to entry is going to be high.

(1) Thank you for the test coverage.

(2) Thank you for giving a specific example with real libraries explaining why this should exist.

(3) How long would we want to live with this? Would this have an expiration date after which we could remove it and again just depend on oauth2client>=2.0.0?

@toabctl
Copy link
Contributor Author

toabctl commented Mar 18, 2016

How long would we want to live with this? Would this have an expiration date after which we could remove it and again just depend on oauth2client>=2.0.0?

As long as possible. But at least as long as other software from google uses still an old oauth2client :)

@theacodes
Copy link
Contributor

@toabctl what software is still using old oauth2client?

@theacodes
Copy link
Contributor

I see above that the cloud sdk does, but it normally vendors its dependencies. Does the debian package actually depend on a separate oauth2client package?

@toabctl
Copy link
Contributor Author

toabctl commented Mar 23, 2016

I see above that the cloud sdk does, but it normally vendors its dependencies. Does the debian package actually depend on a separate oauth2client package?

The openSUSE package does (but Debian is doing the same - removing included 3rd party modules from a package). See https://build.opensuse.org/package/show/openSUSE:Factory/google-cloud-sdk

@theacodes
Copy link
Contributor

Thank you for that information. I'm fine to merge if @nathanielmanistaatgoogle is.

@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit f99fa88 into googleapis:master Mar 23, 2016
akrherz pushed a commit to akrherz/google-api-python-client that referenced this pull request Apr 1, 2019
Also
- Removing bare `except:` statements in both the PyCrypto and
  OpenSSL verifiers (in the `verify` method).
- Catching the only exception possible in the OpenSSL verifier
  (it is `OpenSSL.crypto.Error`).
- Converting the signature to bytes (if not already) in the
  OpenSSL verifier.
- Adding a test with both a unicode and bytes signature for
  each verifier.

Fixes googleapis#201.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants