kaxil commented on PR #63558: URL: https://github.com/apache/airflow/pull/63558#issuecomment-4058453154
This PR targets main, but the problem it describes doesn't exist on either main or 3.1.8. **On main:** `_run_upgradedb()` already calls both `add_default_pool_if_not_exists()` and `synchronize_log_template()` unconditionally: https://github.com/apache/airflow/blob/a9a9c5a57fbaaa11499967b39bac11adedea857e/airflow-core/src/airflow/utils/db.py#L1171-L1182 So the fresh-DB-with-explicit-revision path (`upgradedb()` skips `initdb()`, falls through to `_run_upgradedb()`) already creates the default pool. This PR adds a duplicate call. **On 3.1.8** (the version from the linked issue), the fresh-DB check in `upgradedb()` doesn't even look at `to_revision`: https://github.com/apache/airflow/blob/3.1.8/airflow-core/src/airflow/utils/db.py#L1122-L1126 Every fresh database goes through `initdb()` → `add_default_pool_if_not_exists()`: https://github.com/apache/airflow/blob/3.1.8/airflow-core/src/airflow/utils/db.py#L753-L755 And for existing databases, `add_default_pool_if_not_exists()` is called after the migration: https://github.com/apache/airflow/blob/3.1.8/airflow-core/src/airflow/utils/db.py#L1153-L1154 The test in this PR passes because it mocks `_run_upgradedb` entirely (so the internal `add_default_pool_if_not_exists` call never happens), then asserts the new external call fires. In real code, `_run_upgradedb` already handles it. See my comment on #63551 for the full analysis. -- 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]
