-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
[AIRFLOW-6095] Filter dags returned by task_stats #6684
Conversation
d918bef
to
60bd3aa
Compare
Codecov Report
@@ Coverage Diff @@
## master #6684 +/- ##
==========================================
- Coverage 83.88% 83.88% -0.01%
==========================================
Files 668 668
Lines 37684 37694 +10
==========================================
+ Hits 31612 31620 +8
- Misses 6072 6074 +2
Continue to review full report at Codecov.
|
@@ -403,7 +405,7 @@ <h2>DAGs</h2> | |||
container: "body", | |||
}); | |||
}); | |||
d3.json("{{ url_for('Airflow.task_stats') }}", function(error, json) { | |||
d3.json("{{ url_for('Airflow.task_stats') }}?dag_ids=" + all_dags_ids.join(','), function(error, json) { |
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.
is ,
allowed in DAGids? 😁
Also this parameter needs to be URI escaped, or bettwer yet if d3.json
supports it d3.json("{{ url_for('Airflow.task_stats') }}", {"dag_ids": all_dags_ids.join(',') }, function(error, json) {
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.
Passing get parameters like you suggested above wasn't supported by d3.json so instead I've encoded each dag_id. I thought about passing thedag_id=
multiple times but it does result in a rather long GET header (which I believe is often capped at 8kb?). This could potentially overflow for users with 100 dags per page.
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.
I think, we can use request.getlist("dag_ids")
and send all request in seperate parameter e.g. url-view?dag_ids=AAA&dag_ids=AAA
. Parameters should be encoded and so independent.
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.
Hey sorry this got lost in my inbox. I still have to provide a PR for the /blocked endpoint. Happy to provide an additional PR switching the rest of the end points to use getlist() if you guys prefer 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.
See comments
60bd3aa
to
f9f7535
Compare
cbb8982
to
df0c127
Compare
Add dag_ids parameter to task_stats so can filter by a set of dag_ids present on the dags view. This is intended to speed up the response time and reduce the size of the payload when running a large number of dags.
df0c127
to
2b11f55
Compare
/cc @KevinYang21 You might find this useful. |
Add dag_ids parameter to task_stats so can filter by a set of dag_ids present on the dags view. This is intended to speed up the response time and reduce the size of the payload when running a large number of dags. (Merged on to release branch by PR#6730) (cherry picked from commit 2b11f55)
Add dag_ids parameter to task_stats so can filter by a set of dag_ids present on the dags view. This is intended to speed up the response time and reduce the size of the payload when running a large number of dags. (Merged on to release branch by PR#6730) (cherry picked from commit 2b11f55)
Add dag_ids parameter to task_stats so can filter by a set of dag_ids present on the dags view. This is intended to speed up the response time and reduce the size of the payload when running a large number of dags.
Jira
Description
Add dag_ids parameter to task_stats end point so can filter by a set of dag_ids
present on the page. This is intended to speed up the response time
and reduce the size of the payload when running a large number of dags. I've experienced this endpoint taking up to 10 seconds and returning a 8mb (uncompressed) payload for around 1500 dags.
Note I have another branch against 1-10-test should this be approved.
Tests
See test_views.py
Commits
Documentation
In case of new functionality, my PR adds documentation that describes how to use it.
No documentation needed