You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "luoyuliuyin (via GitHub)" <gi...@apache.org> on 2025/01/10 10:06:51 UTC
[PR] fix: log action get the correct request body [airflow]
luoyuliuyin opened a new pull request, #45546:
URL: https://github.com/apache/airflow/pull/45546
### Apache Airflow version
main (development)
### How to reproduce
POST request with `content-type` as `application/json; charset=utf-8`
```
curl -X 'PATCH' \
'https://you_host_com/api/v1/dags/dag_id/dagRuns/run_id' \
-H 'accept: application/json' \
-H 'Content-Type: application/json; charset=utf-8' \
-d '{
"state": "success"
}'
```
### Problem Description
When an API call is made, if the API has the `@action_logging` annotation, the event will be recorded in the db `log` table as follows
### Related Code
Currently, whether the current request contains json_body is determined by judging whether `request.headers.get("content-type")` is equal to `application/json`. The relevant code is as follows
But in many cases, the `content-type` contains other information besides `application/json`, such as `application/json; charset=utf-8`. In this case, the database will not record valid information.
### Solution
So the judgment condition should be changed to include
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: log action get the correct request body [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #45546:
URL: https://github.com/apache/airflow/pull/45546#discussion_r1910284610
##########
tests/api_connexion/endpoints/test_dag_run_endpoint.py:
##########
@@ -1522,6 +1522,63 @@ def test_should_respond_200(self, state, run_type, dag_maker, session):
assert response.status_code == 200
assert response.json == expected_response_json
+ @pytest.mark.parametrize("state", ["failed", "success", "queued"])
+ @pytest.mark.parametrize("run_type", [state.value for state in DagRunType])
+ def test_action_logging(self, state, run_type, dag_maker, session):
+ dag_id = "TEST_DAG_ID"
+ dag_run_id = "TEST_DAG_RUN_ID"
+ with dag_maker(dag_id) as dag:
+ task = EmptyOperator(task_id="task_id", dag=dag)
+ self.app.dag_bag.bag_dag(dag, root_dag=dag)
+ dr = dag_maker.create_dagrun(run_id=dag_run_id, run_type=run_type)
+ ti = dr.get_task_instance(task_id="task_id")
+ ti.task = task
+ ti.state = State.RUNNING
+ session.merge(ti)
+ session.commit()
+
+ request_json = {"state": state}
+
+ self.client.patch(
+ f"api/v1/dags/{dag_id}/dagRuns/{dag_run_id}",
+ json=request_json,
+ environ_overrides={"REMOTE_USER": "test"},
+ )
+
+ from airflow.models import Log
Review Comment:
move imports at the top of the file if possible, or top of the function if cyclic imports are witnessed.
`json` should go at the top
##########
tests/api_connexion/endpoints/test_dag_run_endpoint.py:
##########
@@ -1522,6 +1522,63 @@ def test_should_respond_200(self, state, run_type, dag_maker, session):
assert response.status_code == 200
assert response.json == expected_response_json
+ @pytest.mark.parametrize("state", ["failed", "success", "queued"])
+ @pytest.mark.parametrize("run_type", [state.value for state in DagRunType])
+ def test_action_logging(self, state, run_type, dag_maker, session):
+ dag_id = "TEST_DAG_ID"
+ dag_run_id = "TEST_DAG_RUN_ID"
+ with dag_maker(dag_id) as dag:
+ task = EmptyOperator(task_id="task_id", dag=dag)
+ self.app.dag_bag.bag_dag(dag, root_dag=dag)
+ dr = dag_maker.create_dagrun(run_id=dag_run_id, run_type=run_type)
+ ti = dr.get_task_instance(task_id="task_id")
+ ti.task = task
+ ti.state = State.RUNNING
+ session.merge(ti)
+ session.commit()
+
+ request_json = {"state": state}
+
+ self.client.patch(
+ f"api/v1/dags/{dag_id}/dagRuns/{dag_run_id}",
+ json=request_json,
+ environ_overrides={"REMOTE_USER": "test"},
+ )
+
+ from airflow.models import Log
+
+ log = (
+ session.query(Log)
+ .filter(
+ Log.dag_id == dag_id,
+ Log.run_id == dag_run_id,
+ Log.event == "api.update_dag_run_state",
+ )
+ .order_by(Log.id.desc())
+ .first()
+ )
+ import json
Review Comment:
to move
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: log action get the correct request body [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #45546:
URL: https://github.com/apache/airflow/pull/45546
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: log action get the correct request body [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #45546:
URL: https://github.com/apache/airflow/pull/45546#issuecomment-2583069991
cherry pick PR here:
https://github.com/apache/airflow/pull/45560
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: log action get the correct request body [airflow]
Posted by "luoyuliuyin (via GitHub)" <gi...@apache.org>.
luoyuliuyin commented on PR #45546:
URL: https://github.com/apache/airflow/pull/45546#issuecomment-2582660432
> NIce.
>
> Small nits before merging, but looking good.
Thank you for the suggestion. Please check the latest commit.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: log action get the correct request body [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #45546:
URL: https://github.com/apache/airflow/pull/45546#issuecomment-2583028895
### Backport failed to create: v2-10-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/12712770597'> Run details </a>
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>❌</td>
<td>v2-10-test</td>
<td><a href="https://github.com/apache/airflow/commit/bae4bb1d549f20a54a2e8c27c57377a0207f393b"><img src='https://img.shields.io/badge/Commit-bae4bb1-red' alt='Commit Link'></a></td>
</tr>
</table>
You can attempt to backport this manually by running:
```bash
cherry_picker bae4bb1 v2-10-test
```
This should apply the commit to the v2-10-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.
After you have resolved the conflicts, you can continue the backport process by running:
```bash
cherry_picker --continue
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org