Copilot commented on code in PR #63994:
URL: https://github.com/apache/airflow/pull/63994#discussion_r3066476755
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py:
##########
@@ -157,27 +165,55 @@ def patch_pool(
"",
status_code=status.HTTP_201_CREATED,
responses=create_openapi_http_exception_doc(
- [status.HTTP_409_CONFLICT]
- ), # handled by global exception handler
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_409_CONFLICT, # handled by global exception handler
+ ]
+ ),
dependencies=[Depends(requires_access_pool(method="POST")),
Depends(action_logging())],
)
def post_pool(
body: PoolBody,
session: SessionDep,
) -> PoolResponse:
"""Create a Pool."""
+ if body.team_name is not None and not conf.getboolean("core",
"multi_team"):
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ MULTI_TEAM_ERROR_MESSAGE,
+ )
+
pool = Pool(**body.model_dump())
session.add(pool)
return pool
@pools_router.patch(
"",
+ responses=create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST]),
dependencies=[Depends(requires_access_pool_bulk()),
Depends(action_logging())],
)
def bulk_pools(
request: BulkBody[PoolBody],
session: SessionDep,
) -> BulkResponse:
"""Bulk create, update, and delete pools."""
+ if not conf.getboolean("core", "multi_team"):
+ invalid_entities = []
+
+ for action in request.actions:
+ if action.action == BulkAction.CREATE or action.action ==
BulkAction.UPDATE:
+ for entity in action.entities:
+ if isinstance(entity, PoolBody) and entity.team_name is
not None:
+ invalid_entities.append(entity.pool)
Review Comment:
`PoolBody` entities in this API appear to use the field `name` (the tests
send `name` and assert `invalid_pool_names` like `pool_2`). Appending
`entity.pool` will likely raise an `AttributeError` (500) instead of returning
the intended 400. Use the correct attribute (e.g., `entity.name`) when
collecting invalid pool names.
```suggestion
invalid_entities.append(entity.name)
```
##########
airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx:
##########
@@ -89,6 +89,11 @@ export const useEditConnection = (
if (requestBody.schema !== initialConnection.schema) {
updateMask.push("schema");
}
+ if (requestBody.team_name !== initialConnection.team_name) {
+ updateMask.push("team_name");
+ }
+
+ const teamName = requestBody.team_name === "" ? undefined :
requestBody.team_name;
Review Comment:
The `updateMask` comparison happens before normalizing `team_name` (`\"\"` →
`undefined`). This can cause `team_name` to be added to `updateMask` even when
the effective value is unchanged (e.g., initial `undefined`, form value
`\"\"`). Normalize first (or compare normalized values), then decide whether to
push `team_name` into `updateMask`.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py:
##########
@@ -183,11 +197,31 @@ def post_variable(
@variables_router.patch(
- "", dependencies=[Depends(action_logging()),
Depends(requires_access_variable_bulk())]
+ "",
+ responses=create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST]),
+ dependencies=[Depends(action_logging()),
Depends(requires_access_variable_bulk())],
)
def bulk_variables(
request: BulkBody[VariableBody],
session: SessionDep,
) -> BulkResponse:
"""Bulk create, update, and delete variables."""
+ if not conf.getboolean("core", "multi_team"):
+ invalid_entities = []
+
+ for action in request.actions:
+ if action.action == BulkAction.CREATE or action.action ==
BulkAction.UPDATE:
+ for entity in action.entities:
+ if isinstance(entity, VariableBody) and entity.team_name
is not None:
+ invalid_entities.append(entity.key)
+
+ if invalid_entities:
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ {
+ "message": MULTI_TEAM_ERROR_MESSAGE,
+ "invalid_variable_keys": invalid_entities,
+ },
+ )
Review Comment:
The new 400 responses are documented as `HTTPExceptionResponse`, which
commonly models `detail` as a string. Here (and similarly in pools/connections
bulk) the error `detail` is a structured object (`message`, `invalid_*`). This
can create an OpenAPI/schema mismatch and break generated clients/types.
Consider defining a dedicated response model for this structured 400 (and
referencing it via `responses=...`), or switch to a 422-style validation error
shape that aligns with existing schemas.
--
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]
