pierrejeambrun commented on code in PR #42973:
URL: https://github.com/apache/airflow/pull/42973#discussion_r1804332445
##########
airflow/api_fastapi/routes/public/dag_run.py:
##########
@@ -57,3 +57,19 @@ async def delete_dag_run(dag_id: str, dag_run_id: str,
session: Annotated[Sessio
)
session.delete(dag_run)
+
+
+@dag_run_router.patch("/{dag_run_id}",
responses=create_openapi_http_exception_doc([400, 401, 403, 404]))
Review Comment:
The global idea yes:
- PUT a full object that override the existing or create a new one if it
does not exist
- PATCH -> Update an object partially
By idempotent, what we mean is if you send the request multiple times it
doesn't change the result. For instance sending mulitple times the request to
set the state to `success` will just leave the task in success, so this would
be idempotent.
Then there is how we define things in airflow, indeed we patch partially a
resource by setting just it's state field, so a PATCH would be more
appropriate. And for `PATCH` we could just take a partial payload, and don't
need a submask. But for now in airflow `PATCH` are defined with a submask and a
full payload, and this set_state method used to be a `PUT` because `PATCH`
requires both a full body and a `mask`.
I would say that at first we should just focus on migrating things over, so
I would just do the same as the legacy code.
And in separate PR / discussions we can propose refactoring / improvement /
breaking change that will go beyond the scope of this PR. (For instance if we
decide that the `PATCH` can take partial body, then the `mask` is not needed
anymore, and this route can become a standard `PATCH` which is more in line
with the HTTP spec). But in a separate PR I would suggest, so we remain
consistent for this one.
--
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]
