Vamsi-klu commented on PR #67724: URL: https://github.com/apache/airflow/pull/67724#issuecomment-4584958055
Found a few blockers in the current diff: 1. The migration creates a second Alembic head. `0117_3_3_0_add_task_circuit_breaker.py` revises `acc215baed80`, but `0117_3_3_0_change_deadline_interval_to_json.py` already revises the same parent. CI confirms this with `Multiple heads are present; a10edcba2695, 8812eb67b63c`. 2. The scheduler interval callbacks add unbounded writes/scans. `reset_expired()` performs one bulk update, and `_skip_circuit_breaker_blocked_tis()` selects all scheduled blocked task instances and mutates them in one interval. Scheduler-loop cleanup/write paths need batching with limits and commits between batches. 3. `record_failure()` is not concurrency-safe. It reads the circuit row, increments in memory, and writes later without locking or an atomic upsert/update. Parallel failures for the same `(dag_id, task_id)` can lose increments or race on first insert, so the circuit may not open at the configured threshold. 4. Static checks also catch new positional `session` parameters on `@provide_session` methods. These need keyword-only `session` parameters. 5. `git diff --check` currently fails on trailing whitespace in generated `services.gen.ts`. Please regenerate/fix through the generator path. --- Drafted-by: Codex (GPT-5); reviewed by @Vamsi-klu before posting -- 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]
