Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix compute engine IDTokenCredentials.with_target_audience method - #361

Closed
lmiglio wants to merge 1 commit into
googleapis:masterfrom
lmiglio:bugfix/with_target_audience
Closed

fix compute engine IDTokenCredentials.with_target_audience method#361
lmiglio wants to merge 1 commit into
googleapis:masterfrom
lmiglio:bugfix/with_target_audience

Conversation

@lmiglio

@lmiglio lmiglio commented Jul 30, 2019

Copy link
Copy Markdown
Contributor

Hi everybody,

Here is the PR for this issue: #359

It should be backward compatible and it should not break anything.

I added an integration test of IDTokenCredentials and Signer where I used responses for mocking REST responses, I do not know if that is fine.

I followed these guidelines.

Hope everything is ok, if not let me know :)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 30, 2019
@busunkim96
busunkim96 self-requested a review August 7, 2019 21:31
Comment thread setup.py Outdated
setup(
name='google-auth',
version='1.6.3',
version='1.6.4',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cut separate release PRs to just bump the version and add changelog notes. See #325 for an example.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sorry, did not know that. I will revert the version to the one present before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I squashed the commits in order to have a single commit with the fix (thus avoiding a commit just for reverting the version back, I do not know if this is compliant with your branch strategy). Let me know if there is other I can do. Thanks!

@salrashid123

Copy link
Copy Markdown
Contributor

cross referencing with PR
#362

that'll remove using the iam signing just to get ID tokens on compute...
you can easily an id_token through the metadata server

@lmiglio
lmiglio force-pushed the bugfix/with_target_audience branch from 9e72686 to a84e427 Compare August 19, 2019 09:24

@busunkim96 busunkim96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting this PR!

@bshaffer

bshaffer commented Feb 7, 2020

Copy link
Copy Markdown
Contributor

Hi @lmiglio !
Could you fix the merge conflicts so we can merge this PR? I've resolved them here for example (see #438)

@bshaffer

Copy link
Copy Markdown
Contributor

Superceded by #438

@bshaffer bshaffer closed this Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants