SameerMesiah97 commented on code in PR #64022:
URL: https://github.com/apache/airflow/pull/64022#discussion_r2971807691


##########
task-sdk/tests/task_sdk/execution_time/test_secrets.py:
##########
@@ -207,6 +207,21 @@ def test_server_context_with_env_var(self, monkeypatch):
         assert "MetastoreBackend" in backend_classes
         assert "ExecutionAPISecretsBackend" not in backend_classes
 
+    def test_triggerer_server_context_includes_metastore_backend(self, 
monkeypatch):

Review Comment:
   This looks very similar to `test_server_context_with_env_var` and is 
effectively asserting the same behavior (that 
`_AIRFLOW_PROCESS_CONTEXT="server"` includes `MetastoreBackend`), but with one 
less assertion. It adds little value unless both backends can co-exist, which I 
believe is not plausible. Also, the new behavior introduced in this PR is not 
covered, which is that the triggerer sets `_AIRFLOW_PROCESS_CONTEXT` at runtime.
   
   It might be more useful to add a small test in 
`airflow-core/tests/unit/cli/commands/test_triggerer_command.py` that asserts 
`_AIRFLOW_PROCESS_CONTEXT` is set to `"server"` during `triggerer_run()`.



-- 
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]

Reply via email to