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

[Translate] Warning when using ADC + API Key #11060

Open
clundin25 opened this issue Aug 1, 2024 · 7 comments
Open

[Translate] Warning when using ADC + API Key #11060

clundin25 opened this issue Aug 1, 2024 · 7 comments
Labels
api: translate Issues related to the Cloud Translation API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@clundin25
Copy link

Moving forward API key will have preference over ADC.

I believe this error message is unnecessary and possibly confusing for API key users.

Should it be removed?

"Ignoring Application Default Credentials {0}: using explicit setting for API key instead.",

@burkedavison burkedavison transferred this issue from googleapis/google-cloud-java Aug 5, 2024
@blakeli0
Copy link
Contributor

blakeli0 commented Aug 5, 2024

@clundin25 Thanks for reporting this issue! I agree we should remove this warning. In addition, is the current authentication order correct? In which explicitly set Credentials has preference over API keys.

Also moving this issue back to google-cloud-java as this is a handwritten layer on top of GAPIC(We do have a few of them in the mono repo).

@blakeli0 blakeli0 transferred this issue from googleapis/sdk-platform-java Aug 5, 2024
@blakeli0 blakeli0 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Aug 5, 2024
@clundin25
Copy link
Author

I think the order should be:

    // 1. explicitly set API key
    // 2. explicitly set credentials

It's less clear to me what the ENV variable order should be.

@blakeli0
Copy link
Contributor

blakeli0 commented Aug 5, 2024

@clundin25 Would there be any problem if we change the order? The comment seems to indicate that the backend is expecting this specific order to avoid conflict.

@clundin25
Copy link
Author

I don't think the issue is the order, rather that both the api key and credential are in the same request. If they are not source from the same project, the request will fail.

So as long as the code chooses only one, it should not be a problem.

@piaxc
Copy link

piaxc commented Aug 6, 2024

Is Java the only language that flags an error if the two credentials are from a different project? What do other languages do in this situation-- and what order do they use? The last thing we need is different behavior depending on language. This should be spec'ed out somewhere.

@clundin25
Copy link
Author

Is Java the only language that flags an error if the two credentials are from a different project?

This enforced from the server so it should not work for any language.

What do other languages do in this situation-- and what order do they use?

I think only Python and Go support API keys for GAPIC, and neither support the env variable.

I believe only this library and Ruby reference the "GOOGLE_API_KEY" ENV variable. PHP does seem to use it in a test case.

https://github.com/search?q=org%3Agoogleapis+GOOGLE_API_KEY+-lang%3Amarkdown+-lang%3Ayaml&type=code

@blakeli0
Copy link
Contributor

blakeli0 commented Aug 7, 2024

only this library

Yes, this specific code is in handwritten only for this library. Other GAPIC libraries do not support API keys through client settings yet. We documented authenticating through headers in our README.

@blakeli0 blakeli0 added the api: translate Issues related to the Cloud Translation API. label Aug 7, 2024
@blakeli0 blakeli0 changed the title Warning when using ADC + API Key [Translate] Warning when using ADC + API Key Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: translate Issues related to the Cloud Translation API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants