Copilot commented on code in PR #62964:
URL: https://github.com/apache/airflow/pull/62964#discussion_r2892616294
##########
airflow-core/tests/unit/executors/test_workloads.py:
##########
@@ -25,3 +25,46 @@ def test_task_instance_alias_keeps_backwards_compat():
assert TaskInstance is TaskInstanceDTO
assert workloads.TaskInstance is TaskInstanceDTO
assert workloads.TaskInstanceDTO is TaskInstanceDTO
+
+
+def test_token_excluded_from_workload_repr():
+ """Ensure JWT tokens do not leak into log output via repr().
+
+ Regression test for https://github.com/apache/airflow/issues/62428
Review Comment:
The test docstring references a specific issue URL
(`https://github.com/apache/airflow/issues/62428`). Per project conventions,
test names and docstrings should describe behavior, not track tickets. Consider
changing the docstring to just describe what the test verifies, e.g.,
`"""Ensure JWT tokens do not leak into log output via repr()."""`
##########
airflow-core/tests/unit/executors/test_workloads.py:
##########
@@ -25,3 +25,46 @@ def test_task_instance_alias_keeps_backwards_compat():
assert TaskInstance is TaskInstanceDTO
assert workloads.TaskInstance is TaskInstanceDTO
assert workloads.TaskInstanceDTO is TaskInstanceDTO
+
+
+def test_token_excluded_from_workload_repr():
+ """Ensure JWT tokens do not leak into log output via repr().
+
+ Regression test for https://github.com/apache/airflow/issues/62428
+ """
+ from pathlib import PurePosixPath
+ from uuid import uuid4
+
+ from airflow.executors.workloads.base import BundleInfo
+ from airflow.executors.workloads.task import ExecuteTask, TaskInstanceDTO
Review Comment:
All imports in this test function should be moved to the top of the file.
The codebase convention (see `test_base_executor.py:24`,
`test_base_executor.py:38`, `test_local_executor.py:32-34`) is to place imports
at module level. Standard library imports like `pathlib` and `uuid`, as well as
airflow imports like `BundleInfo`, `ExecuteTask`, and `TaskInstanceDTO`, have
no circular import or lazy-loading justification here. Note that
`TaskInstanceDTO` is already imported at the top of this file (line 21) from a
different path.
--
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]
