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.

Sign up for GitHub

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

Jump to bottom

Allowing pyOpenSSL import to fail for GAE. #1446

Merged
merged 1 commit into from
Feb 12, 2016
Merged

Allowing pyOpenSSL import to fail for GAE. #1446

merged 1 commit into from
Feb 12, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 11, 2016

Fixes #1445.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 11, 2016
@theacodes
Copy link
Contributor

@dhermes ping me when you're ready for review.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 11, 2016

I am "ready" for review. The AppVeyor fail is noted in #1434

IMO, the coverage drop just needs a pragma no cover. WDYT?

@theacodes
Copy link
Contributor

I think it'd be nice to see a test case for this, if possible.

@theacodes theacodes self-assigned this Feb 11, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

@jonparrott I have previously implemented tests that mock import failures and the pay-off does not justify the amount that goes in.

Maybe there is a better way and I just don't know how to do it?

@theacodes
Copy link
Contributor

I guess I more want to ensure that _get_pem_key and _get_signature_bytes raises a more useful error than an AttributeError if crypto is not available.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

OK cool I can add that feature and test for it.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

@jonparrott PTAL. (Also remind me to squash to the commits before merging.)

theacodes
theacodes reviewed Feb 12, 2016
View reviewed changes
gcloud/credentials.py
@@ -181,7 +187,11 @@ def _get_pem_key(credentials):
raise TypeError((credentials,
'not a valid service account credentials type'))

return crypto.load_privatekey(crypto.FILETYPE_PEM, pem_text)
if crypto is None:
raise EnvironmentError('pyOpenSSL must be installed to load a '

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@theacodes
Copy link
Contributor

LGTM with minor nits. You can squash commits if you want, but I don't see an issue with these being separate commits.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

@jonparrott PTAL

theacodes
theacodes reviewed Feb 12, 2016
View reviewed changes
gcloud/credentials.py
@@ -207,6 +217,9 @@ def _get_signature_bytes(credentials, string_to_sign):
pkey = _get_pem_key(credentials)
if not isinstance(string_to_sign, six.binary_type):
string_to_sign = string_to_sign.encode('utf-8')
if crypto is None:
raise EnvironmentError('pyOpenSSL must be installed to sign '

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@theacodes
Copy link
Contributor

Good to go with the tiniest little nit. Feel free to merge once that's fixed and travis is happy.

dhermes added a commit that referenced this pull request Feb 12, 2016
@dhermes
Merge pull request #1446 from dhermes/fix-1445
ad88be0 
Allowing pyOpenSSL import to fail for GAE.
@dhermes dhermes merged commit ad88be0 into googleapis:master Feb 12, 2016
@dhermes dhermes deleted the fix-1445 branch February 12, 2016 19:28
@dhermes dhermes mentioned this pull request Feb 17, 2016
Merged
atulep pushed a commit that referenced this pull request Apr 6, 2023
@dizcology @busunkim96
add Speech API auto punctuation sample [(#1446)](GoogleCloudPlatform/…
ddd2b69 
…python-docs-samples#1446)

* add auto punctuation sample

* correct docstring
atulep pushed a commit that referenced this pull request Apr 6, 2023
@dizcology @busunkim96
add Speech API auto punctuation sample [(#1446)](GoogleCloudPlatform/…
a0d9c55 
…python-docs-samples#1446)

* add auto punctuation sample

* correct docstring
atulep pushed a commit that referenced this pull request Apr 18, 2023
@dizcology @busunkim96
add Speech API auto punctuation sample [(#1446)](GoogleCloudPlatform/…
95bcc3c 
…python-docs-samples#1446)

* add auto punctuation sample

* correct docstring
parthea pushed a commit that referenced this pull request Oct 22, 2023
@dizcology @busunkim96
add Speech API auto punctuation sample [(#1446)](GoogleCloudPlatform/…
8e49fe8 
…python-docs-samples#1446)

* add auto punctuation sample

* correct docstring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@theacodes theacodes

Labels
auth cla: yes This human has signed the Contributor License Agreement. packaging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhermes @theacodes @googlebot