Skip to content

Conversation

@XD-DENG
Copy link
Member

@XD-DENG XD-DENG commented Dec 27, 2018

Jira

https://issues.apache.org/jira/browse/AIRFLOW-3576

Description

  • Here are some details about my PR, including screenshots of any UI changes:

In airflow/www/templates/airflow/dag.html or airflow/www_rbac/templates/airflow/dag.html, in line <a href="{{ url_for("airflow.delete", dag_id=dag.dag_id, root=root) }}", root is not necessary, given it's not used anywhere in the '/delete' method in either airflow/www/views.py or airflow/www_rbac/views.py.

Please correct me if I'm incorrect about this or missed anything.

@XD-DENG
Copy link
Member Author

XD-DENG commented Dec 27, 2018

@Noremac201 may you advise on this PR as you authored the original commit (#3531)?

Kindly let me know if I have misunderstood or missed anything in your implementation.

Thanks!

'root' is not used anywhere in `delete` method in either
www/views.py or www_rbac/views.py.

Having it in url_for("airflow.delete", dag_id=dag.dag_id, root=root)
in dag.html is meaningless.
@XD-DENG XD-DENG force-pushed the remove_unnecessary_arg_root branch from 3082886 to 1e87643 Compare December 27, 2018 14:58
@XD-DENG
Copy link
Member Author

XD-DENG commented Dec 27, 2018

1 test (out of 9) failed due to transient error.

@Noremac201
Copy link
Contributor

Lgtm👍

@kaxil
Copy link
Member

kaxil commented Dec 27, 2018

have restarted the test

@codecov-io
Copy link

Codecov Report

Merging #4380 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4380      +/-   ##
==========================================
- Coverage   78.16%   78.16%   -0.01%     
==========================================
  Files         204      204              
  Lines       16530    16530              
==========================================
- Hits        12921    12920       -1     
- Misses       3609     3610       +1
Impacted Files Coverage Δ
airflow/models/__init__.py 92.76% <0%> (-0.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cc078a...1e87643. Read the comment docs.

@XD-DENG
Copy link
Member Author

XD-DENG commented Dec 31, 2018

Hi @kaxil , a gentle ping. The tests have passed.

@feng-tao
Copy link
Member

feng-tao commented Jan 2, 2019

LGTM

@feng-tao feng-tao merged commit 7ee9266 into apache:master Jan 2, 2019
@XD-DENG XD-DENG deleted the remove_unnecessary_arg_root branch January 2, 2019 06:50
aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
…pache#4380)

'root' is not used anywhere in `delete` method in either
www/views.py or www_rbac/views.py.

Having it in url_for("airflow.delete", dag_id=dag.dag_id, root=root)
in dag.html is meaningless.
yohei1126 pushed a commit to yohei1126/airflow that referenced this pull request Jan 6, 2019
…pache#4380)

'root' is not used anywhere in `delete` method in either
www/views.py or www_rbac/views.py.

Having it in url_for("airflow.delete", dag_id=dag.dag_id, root=root)
in dag.html is meaningless.
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
…pache#4380)

'root' is not used anywhere in `delete` method in either
www/views.py or www_rbac/views.py.

Having it in url_for("airflow.delete", dag_id=dag.dag_id, root=root)
in dag.html is meaningless.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants