-
Notifications
You must be signed in to change notification settings - Fork 14.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce response payload size of /dag_stats and /task_stats #8633
Reduce response payload size of /dag_stats and /task_stats #8633
Conversation
I tested and UI is working properly after applying this change. Another duplicated information in the response payload is the |
Should we write a note in UPDATING.md? Because it's public facing and some people might be using it for whatever reason? |
Hi @BasPH , thanks for the reminding. Earlier I was thinking that it's not directly used by users (only called by UI code) so no need to update Will add a commit here to update it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for this?
Hi @BasPH and @kaxil , updated as suggested. There are failure in tests May you please help advise? Sorry I'm not very familiar with current CI setup 😅 |
Hey @XD-DENG -> It is an interesting thing and it should not happen. I will take a look at that (and likely will have to fix something). This enum34 appearing in generate-requirements should not be there. I will let you know when i fix it (looking at it now.). |
Thanks @potiuk ! Appreciate it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There is a test failure about JSON response too:
|
ed730e5
to
44eec69
Compare
Their response format is like {"example_dag_id": [{"state": "success", "dag_id": "example_dag_id"}, ...], ...} The dag_id is already used as the "key", but still repeatedly appear in each element, which makes the response payload size unnecessarily bigger
44eec69
to
c85fb20
Compare
Hi @potiuk , I have noticed your latest CI fix. Have rebased here. |
Their response payload of
/dag_stats
and/task_stats
are likeThe
dag_id
is already present as the key, but still repeatedly appear in each element,which makes the response payload size unnecessarily bigger.
In one of the tests I did, this change reduces the response payload size of
/task_stats
to 23Kb from 33Kb.Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.