pierrejeambrun commented on code in PR #48109:
URL: https://github.com/apache/airflow/pull/48109#discussion_r2013802759
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py:
##########
@@ -569,7 +608,7 @@ def test_patch_should_respond_404(self, test_client, body):
@pytest.mark.enable_redact
@pytest.mark.parametrize(
- "body, expected_response",
+ "body, expected_response, update_mask",
Review Comment:
I don't think we need the update_mask update.
All fields that are wet will be updated.
##########
airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py:
##########
@@ -34,6 +34,26 @@
from airflow.models.connection import Connection
+def update_orm_from_pydantic(
+ orm_conn: Connection, pydantic_conn: ConnectionBody, update_mask:
list[str] | None = None
+):
Review Comment:
return type annotation
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py:
##########
@@ -187,29 +190,20 @@ def patch_connection(
"The connection_id in the request body does not match the URL
parameter",
)
- non_update_fields = {"connection_id", "conn_id"}
- connection =
session.scalar(select(Connection).filter_by(conn_id=connection_id).limit(1))
+ connection: Connection =
session.scalar(select(Connection).filter_by(conn_id=connection_id).limit(1))
if connection is None:
raise HTTPException(
status.HTTP_404_NOT_FOUND, f"The Connection with connection_id:
`{connection_id}` was not found"
)
- fields_to_update = patch_body.model_fields_set
-
- if update_mask:
- fields_to_update = fields_to_update.intersection(update_mask)
- else:
- try:
- ConnectionBody(**patch_body.model_dump())
- except ValidationError as e:
- raise RequestValidationError(errors=e.errors())
-
- data = patch_body.model_dump(include=fields_to_update - non_update_fields,
by_alias=True)
-
- for key, val in data.items():
- setattr(connection, key, val)
+ try:
+ ConnectionBody(**patch_body.model_dump())
+ except ValidationError as e:
+ raise RequestValidationError(errors=e.errors())
Review Comment:
This needs to be moved inside the `update_orm_from_pydantic` I guess.
It needs to be done only when there is no update mask. (This check is weird,
I think it's some old remnant of refactoring and we should probably update that
in all `PATCH` endpoint, but for an other PR)
--
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]
