potiuk commented on code in PR #61036:
URL: https://github.com/apache/airflow/pull/61036#discussion_r2933870179
##########
airflow-core/src/airflow/cli/commands/dag_processor_command.py:
##########
@@ -35,7 +36,15 @@
def _create_dag_processor_job_runner(args: Any) -> DagProcessorJobRunner:
"""Create DagFileProcessorProcess instance."""
if args.bundle_name:
Review Comment:
nOte: `_create_dag_processor_job_runner` is called from within the
`dag_processor` function, which is already wrapped by `@cli_utils.action_cli`.
The `action_cli` decorator runs `check_and_run_migrations()` (when
`check_db=True`) before calling the wrapped function, so the DB is available at
this point. The issue is specifically that `_AIRFLOW_PROCESS_CONTEXT` isn't
set, which affects the Task SDK's secret backend resolution.
This is fine as-is, but a comment explaining *why* the context needs to be
set here would help future readers understand the dependency on
`MetastoreBackend`.
##########
airflow-core/tests/unit/cli/commands/test_dag_processor_command.py:
##########
@@ -57,6 +57,29 @@ def test_bundle_names_passed(self, mock_runner,
configure_testing_dag_bundle):
dag_processor_command.dag_processor(args)
assert mock_runner.call_args.kwargs["processor"].bundle_names_to_parse
== ["testing"]
+ @conf_vars({("core", "load_examples"): "False"})
+
@mock.patch("airflow.cli.commands.dag_processor_command.DagProcessorJobRunner")
+ @mock.patch("airflow.utils.cli.DagBundlesManager", autospec=True)
+ def test_bundle_validation_runs_with_server_context(self,
mock_manager_cls, mock_runner):
+ mock_runner.return_value.job_type = "DagProcessorJob"
+ captured_ctx = {}
+
+ mock_bundle = mock.MagicMock()
+ mock_bundle.name = "bundle1"
+
+ def capture_env_and_return_bundles():
+ captured_ctx["during_validation"] =
os.environ.get("_AIRFLOW_PROCESS_CONTEXT")
+ return [mock_bundle]
+
+ mock_manager_cls.return_value.get_all_dag_bundles.side_effect =
capture_env_and_return_bundles
+
+ os.environ.pop("_AIRFLOW_PROCESS_CONTEXT", None)
Review Comment:
**Bug: Environment variable leak risk in test.**
`os.environ.pop("_AIRFLOW_PROCESS_CONTEXT", None)` modifies the real
`os.environ` without restoring the original value after the test. If another
test in the same process previously set this variable, this test silently
removes it. Use `monkeypatch.delenv` or wrap the mutation in a try/finally:
```python
def test_bundle_validation_runs_with_server_context(self, mock_manager_cls,
mock_runner, monkeypatch):
...
monkeypatch.delenv("_AIRFLOW_PROCESS_CONTEXT", raising=False)
...
```
Also, the assertion on line 81 (`assert "_AIRFLOW_PROCESS_CONTEXT" not in
os.environ`) verifies cleanup, but if the test itself fails before that line,
the env var could remain set and leak into subsequent tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
