Airflow poc - #2181
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Managed Airflow DAG package (import-automation/airflow) designed to run parameterized container commands as Cloud Batch jobs. It includes the DAG definition, helper modules for job configuration, unit tests, and comprehensive documentation. The review feedback highlights an important Airflow anti-pattern where environment variables are validated at the top level of the DAG file, which can cause DAG import errors during periodic parsing. It is recommended to defer this validation to runtime or handle missing variables gracefully.
| PROJECT_ID = get_required_environment(os.environ, PROJECT_ID_ENV) | ||
| REGION = get_required_environment(os.environ, REGION_ENV) | ||
| SERVICE_ACCOUNT_EMAIL = get_required_environment(os.environ, | ||
| SERVICE_ACCOUNT_ENV) |
There was a problem hiding this comment.
Evaluating environment variables and raising exceptions at the top level of a DAG file is an Airflow anti-pattern. The DAG parser executes this file periodically (every few seconds) in multiple components (scheduler, webserver, etc.). If these environment variables are missing in any of those environments (e.g., during webserver startup, local testing, or linting), the entire DAG import will fail with a ValueError, causing a prominent DAG Import Error in the UI and potentially blocking other DAGs.
Instead, retrieve these environment variables using os.environ.get() with a default or empty string, and let the operator or a runtime task validate them during execution.
PROJECT_ID = os.environ.get(PROJECT_ID_ENV, '')
REGION = os.environ.get(REGION_ENV, '')
SERVICE_ACCOUNT_EMAIL = os.environ.get(SERVICE_ACCOUNT_ENV, '')…ud Batch job ID readability and log identification
1 participant
No description provided.