Adding common sign_blob() service account types. #421
Conversation
| @property | ||
| def service_account_email(self): | ||
| """Get the email for the current service account.""" | ||
| _abstract() |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
dhermes
force-pushed
the
sign-blob-all-svc-accounts
branch
3 times, most recently
from
59f1853
to
eae8ba6
Compare
| @property | ||
| def service_account_email(self): | ||
| """Get the email for the current service account.""" | ||
| raise NotImplementedError |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
dhermes
force-pushed
the
sign-blob-all-svc-accounts
branch
from
eae8ba6
to
d1f3d58
Compare
|
@nathanielmanistaatgoogle Removed the abstract |
| tuple, A pair of the private key ID used to sign the blob and | ||
| the signed contents. | ||
| """ | ||
| raise NotImplementedError |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
(Completed a full review pass.) |
dhermes
force-pushed
the
sign-blob-all-svc-accounts
branch
from
d1f3d58
to
90cc579
Compare
| content = _from_bytes(content) | ||
| return content | ||
| else: | ||
| raise RuntimeError(response, content) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Looks good except for the copied-and-pasted doc strings. (EDIT: Oops, and your comment about |
| content = _from_bytes(content) | ||
| return None, content | ||
| else: | ||
| return response, content |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@nathanielmanistaatgoogle See the 4th commit for the "implements" note. |
|
All looks great; squash and merge. Thanks for the conversation. |
Also adding service_account_email() property.
dhermes
force-pushed
the
sign-blob-all-svc-accounts
branch
from
d402118
to
ce0d71a
Compare
|
👍 Will wait to merge until Travis is done in 10 hours. |
Adding common sign_blob() service account types.
Also adding
service_account_email()property.See googleapis/google-cloud-python#922 for some details on why
sign_blobwon't work on GCE.I'm a bit worried about
service_account_idon the GAE credentials class. In the implementation it seems to be almost synonymous with the svc. account email / name, but not quite (i.e. there may be some legacy behavior). ISTM the reason it's in the constructor is so that it could be passed toapp_identity. Anyhow, ourservice_account_namecould either shadow or conflict withservice_account_idif a user passed one in. Though I don't imagine anyone in the history of this library has passed in a value forservice_account_id.Context: We use these utilities in a haphazard way in
gcloud-pythonwhen creating a signed URL (for cloud storage objects). This just puts the auth utils in the auth library in a unified way./cc @tseaver @jgeewax @jonparrott