Copilot commented on code in PR #60108:
URL: https://github.com/apache/airflow/pull/60108#discussion_r3066491837
##########
airflow-core/src/airflow/api_fastapi/execution_api/app.py:
##########
@@ -311,9 +319,13 @@ class InProcessExecutionAPI:
@cached_property
def app(self):
if not self._app:
+ import svcs
+
+ from airflow.api_fastapi.auth.tokens import JWTGenerator
Review Comment:
`InProcessExecutionAPI.app` adds new in-method imports (`import svcs` /
`from airflow.api_fastapi.auth.tokens import JWTGenerator`). Both modules are
already imported at module scope in this file, so these imports are redundant
and also violate the project guideline to keep imports at the top of the file.
Please remove these local imports (and, if `_container` must stay local for a
circular-import reason, add a brief comment explaining why).
```suggestion
```
##########
airflow-core/src/airflow/api_fastapi/execution_api/app.py:
##########
@@ -76,6 +77,7 @@ def _jwt_generator():
generator = JWTGenerator(
valid_for=conf.getint("execution_api", "jwt_expiration_time"),
+ # workload_valid_for uses the attrs default factory which reads the
same config key
Review Comment:
The inline comment says `workload_valid_for ... reads the same config key`,
but `JWTGenerator.workload_valid_for` is sourced from
`execution_api.jwt_workload_token_expiration_time` (a different key than
`jwt_expiration_time`). This comment is misleading and should be updated to
reference the correct config key (or removed).
```suggestion
# workload_valid_for uses the attrs default factory, which reads
# execution_api.jwt_workload_token_expiration_time
```
--
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]
