-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add EE service account authentication #2334
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -76,6 +76,20 @@ def ee_initialize( | |||||
| if ee.data._get_state().credentials is not None: | ||||||
| return | ||||||
|
|
||||||
| if get_env_var("EE_SERVICE_ACCOUNT") is not None: | ||||||
|
|
||||||
| key_data = get_env_var("EE_SERVICE_ACCOUNT") | ||||||
|
|
||||||
| try: | ||||||
| email = json.loads(key_data)["client_email"] | ||||||
| except json.JSONDecodeError as e: | ||||||
| raise ValueError(f"Invalid JSON for key_data: {e}") | ||||||
|
||||||
| raise ValueError(f"Invalid JSON for key_data: {e}") | |
| raise ValueError(f"Invalid JSON in EE_SERVICE_ACCOUNT environment variable: {e}") |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message references key_data which is an internal variable name. For better user experience, consider referencing the environment variable name instead: "EE_SERVICE_ACCOUNT JSON does not contain 'client_email'"
| raise ValueError("key_data JSON does not contain 'client_email'") | |
| raise ValueError("EE_SERVICE_ACCOUNT JSON does not contain 'client_email'") |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ee.Initialize() call doesn't pass the **kwargs parameter, unlike the other authentication paths (e.g., line 106). This means users cannot pass additional parameters like opt_url for the High-Volume platform when using service account authentication. Consider changing to ee.Initialize(credentials, **kwargs) for consistency.
| ee.Initialize(credentials) | |
| ee.Initialize(credentials, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable
key_datais redundantly assigned. Line 79 already checks thatget_env_var("EE_SERVICE_ACCOUNT")is not None, so the result can be stored directly without callingget_env_varagain. Consider storing it in a variable on line 79 to avoid the duplicate call.