Fix data-science agent deployment failure with None environment varia… #392
Conversation
…bles - Filter out None and empty string values from env_vars dictionary - Add logging for skipped environment variables - Maintain backward compatibility with empty strings in .env files - Fixes TypeError when BQML_RAG_CORPUS_NAME and CODE_INTERPRETER_EXTENSION_NAME are empty Fixes google#192
Co-authored-by: rajeshkanaka <[email protected]>
- Default ROOT_AGENT_MODEL to gemini-2.5-flash - Filter empty env vars with aggregated logging before deploy - Raise UsageError for missing flags and tidy agent deletion message
…0201-4c7c-bd6b-fd41c3bf0178
Removed 'NL2SQL_METHOD' from the list of environment variables.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes deployment failures for data-science agents caused by None and empty environment variables. The main issue was a TypeError occurring when BQML_RAG_CORPUS_NAME and CODE_INTERPRETER_EXTENSION_NAME environment variables were empty.
- Filter out None, empty, and whitespace-only values from environment variables before deployment
- Replace print statements with proper
app.UsageErrorexceptions for better error handling - Add comprehensive logging for environment variable processing and skipped values
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/agents/data-science/deployment/deploy.py | Refactored environment variable handling with filtering logic, improved error handling with exceptions, and added detailed logging |
| python/agents/data-science/data_science/agent.py | Added default fallback value for ROOT_AGENT_MODEL environment variable |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| for key in env_var_keys: | ||
| value = os.getenv(key) | ||
| # Only add to env_vars if the value has actual content (not None, empty, or whitespace-only) | ||
| if value and value.strip(): | ||
| env_vars[key] = value | ||
| logger.info("Including environment variable: %s", key) | ||
| else: | ||
| logger.info("Skipping empty/None/whitespace-only environment variable: %s (value: %s)", key, repr(value)) | ||
|
|
||
| logger.info("Environment variables to be passed to agent: %s", list(env_vars.keys())) | ||
|
|
||
| skipped_vars: list[str] = [] | ||
| for key in env_var_keys: | ||
| value = os.getenv(key) | ||
| # Only add to env_vars if the value is not None/empty and not just | ||
| # whitespace | ||
| if value and value.strip(): | ||
| env_vars[key] = value | ||
| else: | ||
| skipped_vars.append(key) | ||
|
|
||
| if skipped_vars: | ||
| logger.info( | ||
| "Skipped empty/None/whitespace environment variables: %s", | ||
| skipped_vars, | ||
| ) | ||
|
|
||
| logger.info( | ||
| "Environment variables to be passed to agent: %s", list(env_vars.keys()) | ||
| ) |
There was a problem hiding this comment.
The environment variable processing logic is duplicated. The first loop (lines 207-214) and second loop (lines 219-226) perform identical filtering. Remove the duplication by keeping only one loop and consolidating the logging.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Rajesh Pandhare <[email protected]>
2 participants
…bles
Fixes #192