Copilot commented on code in PR #62280:
URL: https://github.com/apache/airflow/pull/62280#discussion_r3066474244
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -140,7 +140,7 @@ services:
<<: *airflow-common
command: scheduler
healthcheck:
- test: ["CMD", "curl", "--fail", "http://localhost:8974/health";]
+ test: ["CMD-SHELL", 'airflow jobs check --job-type SchedulerJob
--hostname "$${HOSTNAME}"']
Review Comment:
`airflow jobs check` depends on Airflow being able to reach the metadata DB
and can report “unhealthy” during transient DB connectivity issues (even if the
scheduler process is running). If the intent of this healthcheck is process
liveness rather than end-to-end readiness, consider documenting this tradeoff
in the compose file (or use a liveness-style check) so operators understand why
the scheduler might flip to `unhealthy`.
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -140,7 +140,7 @@ services:
<<: *airflow-common
command: scheduler
healthcheck:
- test: ["CMD", "curl", "--fail", "http://localhost:8974/health";]
+ test: ["CMD-SHELL", 'airflow jobs check --job-type SchedulerJob
--hostname "$${HOSTNAME}"']
Review Comment:
The healthcheck command uses nested quoting (YAML single-quoted string
containing shell double quotes plus Compose `$` escaping), which is hard to
read/maintain and easy to mis-edit. Consider switching the command string to
YAML double quotes (escaping inner quotes as needed) or avoiding the nested
quotes by using a shell substitution like `--hostname "$(hostname)"` (or
omitting `--hostname` if the CLI defaults to the current host), so the escaping
rules are simpler and clearer.
--
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]
