sahilSENPAII opened a new pull request, #76566:
URL: https://github.com/apache/airflow/pull/76566

   <!-- SPDX-License-Identifier: Apache-2.0
         https://www.apache.org/licenses/LICENSE-2.0 -->
   
   ### What breaks
   
   `PATCH /api/v2/pools` deletes `default_pool` when a `delete` action names 
it, and reports it as a success. `DELETE /api/v2/pools/default_pool` refuses 
the same operation with a 400, and `Pool.delete_pool` has the same guard, but 
the bulk service calls `session.delete()` directly and bypasses both.
   
   ### Impact
   
   The scheduler stops queuing tasks in `default_pool`, which is where tasks go 
unless they name another pool. It treats the pool as non-existent and logs 
"Tasks using non-existent pool 'default_pool' will not be scheduled". Airflow 
only recreates the pool from the DB init and upgrade paths.
   
   ### Repro request
   
   ```
   PATCH /api/v2/pools
   {"actions": [{"action": "delete", "entities": ["default_pool"]}]}
   ```
   
   Before this change:
   
   ```
   200 {"create": null, "update": null, "delete": {"success": ["default_pool"], 
"errors": []}}
   ```
   
   The row is gone, and `GET /api/v2/pools/default_pool` returns 404.
   
   ### Fix
   
   `handle_bulk_delete` now raises the same 400, with the same message as the 
single delete route, through the service's existing error handling. It uses the 
existing `Pool.DEFAULT_POOL_NAME` constant.
   
   This follows how the action already reports missing pools: the error goes in 
`errors`, and nothing in that action is deleted, including other pools named 
alongside `default_pool`. The check also applies with `action_on_non_existence: 
skip`, because that setting is about missing pools.
   
   ```
   200 {"create": null, "update": null, "delete": {"success": [], "errors": 
[{"error": "Default Pool can't be deleted", "status_code": 400}]}}
   ```
   
   Bulk `create` and `update` are unchanged. A bulk `create` with `overwrite` 
can still edit `default_pool` fields that the single PATCH refuses. This PR 
leaves that alone because `airflowctl pools import` uses that action.
   
   ### Tests
   
   Added `TestBulkPools::test_bulk_delete_rejects_default_pool` in 
`airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py`. It 
covers `default_pool` alone, `default_pool` alone with `skip`, and 
`default_pool` together with another pool. Each case checks the error response 
and that every named pool still exists. All three fail without the fix.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code (Opus 5.5) following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   
   ---
   
   * Read the **[Pull Request 
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
 for more information. Note: commit author/co-author name and email in commits 
become permanently public when merged.
   * For fundamental code changes, an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
 is needed.
   * When adding dependency, check compliance with the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   * For significant user-facing changes create newsfragment: 
`{pr_number}.significant.rst`, in 
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
 You can add this file in a follow-up commit after the PR is created so you 
know the PR number.
   


-- 
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]

Reply via email to