Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Try token attr fix for compute engine service account #100

Merged

Conversation

lucienfregosibodyguard
Copy link
Contributor

@lucienfregosibodyguard lucienfregosibodyguard commented Dec 6, 2022

Add method + refresh credentials to get the token

Closes #92

Checklist

  • This pull request references any related issue by including "Closes #<ISSUE_NUMBER>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • Summarized PR's changes in CHANGELOG.md

@desertaxle
Copy link
Member

Thanks for the contribution @lucienfregosibodyguard! It looks like some of the tests need to be updated to account for the changes in this PR. Also, would you be able to add an entry to the changelog describing these changes?

@ahuang11
Copy link
Contributor

ahuang11 commented Dec 6, 2022

Thanks for looking into this and making a PR! Looking at the changes, I realized we already have a built-in method for this on GcpCredentials. I think we should call it, but it's currently async so I made it sync compatible here.

Also, we might want to decorate get_configs here with @sync_compatible too and call await self_copy.credentials.get_access_token().

Let me know if that makes sense!

@lucienfregosibodyguard
Copy link
Contributor Author

@ahuang11 it totally makes sense you can close this PR then and use only the original one

Can't wait to have this merged

@ahuang11
Copy link
Contributor

ahuang11 commented Dec 7, 2022

Sure; I'll first merge this PR into mine so you're part of the contributors. Thanks again!

@ahuang11 ahuang11 merged commit 5206140 into PrefectHQ:try-token-attr Dec 7, 2022
desertaxle added a commit that referenced this pull request Dec 7, 2022
* Try to get attributes else do not

* Simplify logic

* fix compute engine credentials method (#100)

* Add tests and cleanup

Co-authored-by: Lucien Fregosi <lucien.fregosi@bodyguard.ai>
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
ahuang11 pushed a commit that referenced this pull request Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants