You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/01/06 18:52:09 UTC
[GitHub] [airflow] IKholopov opened a new pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
IKholopov opened a new pull request #20730:
URL: https://github.com/apache/airflow/pull/20730
related: #19944
- Rename "tree" view to "grid" view
- Update URL routes and add redirects from old path:
- `/tree` -> `/dags/<dag_id>/grid`
- `/graph` -> `/dags/<dag_id>/graph`
- `/landing_times` -> `/dags/<dag_id>/landing_times`
- `/duration` -> `/dags/<dag_id>/duration`
- `/tries` -> `/dags/<dag_id>/tries`
- `/calendar` -> `/dags/<dag_id>/calendar`
- `/gantt` -> `/dags/<dag_id>/gantt`
- `/code` -> `/dags/<dag_id>/code`
- `/dag_details` -> `/dags/<dag_id>/details`
- New redirect - `/dags/<dag_id>` -> `/dags/<dag_id>/grid`
Once a new single DAG page is ready, the subroutes can be dropped and transformed into redirects to single `/dags/<dag_id>` page (with proper query params). As alternative, only single view could've been renamed (`/tree` -> `/dags/<dag_id>`), but I thought that it would be better to keep routes self-consistent mid-flight (in case if single-page view won't make it into 2.3.0).
DAGs page before: 
DAGs page after: 
---
--
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
[GitHub] [airflow] IKholopov commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r796097962
##########
File path: airflow/www/decorators.py
##########
@@ -34,6 +34,15 @@
logger = logging.getLogger(__name__)
+def _get_dag_id() -> str:
+ dag_id = request.args.get('dag_id')
+ if dag_id:
+ return dag_id
+ path_parts = request.path.split('/')
+ if len(path_parts) >= 3 and path_parts[1] == 'dags':
+ return path_parts[2]
Review comment:
Thanks! I was looking for it in a wrong version of Flask, but failed to locate `request.view_args` there for some reason. Fixed.
--
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
[GitHub] [airflow] eladkal commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1026629558
What this means for people who have
`dag_default_view = tree` in `airflow.cfg`?
https://github.com/apache/airflow/blob/4a73d8f3d1f0c2cb52707901f9e9a34198573d5e/airflow/config_templates/default_airflow.cfg#L578-L579
--
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
[GitHub] [airflow] uranusjr merged pull request #20730: Modernize DAG-related URL routes and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #20730:
URL: https://github.com/apache/airflow/pull/20730
--
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
[GitHub] [airflow] IKholopov commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r797073635
##########
File path: airflow/www/views.py
##########
@@ -2788,11 +2895,22 @@ def tries(self, session=None):
]
)
@action_logging
+ def legacy_landing_times(self):
+ """Redirect from url param."""
+ return redirect(url_for('Airflow.landing_times', **request.args))
+
+ @expose('/dags/<string:dag_id>/landing_times')
Review comment:
Done!
--
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
[GitHub] [airflow] IKholopov commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r796105210
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -31,15 +31,15 @@
<meta name="paused_url" content="{{ url_for('Airflow.paused') }}">
<meta name="status_filter" content="{{ status_filter }}">
<meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}">
- <meta name="graph_url" content="{{ url_for('Airflow.graph') }}">
+ <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}">
<meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}">
<meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}">
<meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}">
<meta name="csrf_token" content="{{ csrf_token() }}">
<meta name="last_dag_runs_url" content="{{ url_for('Airflow.last_dagruns') }}">
<meta name="dag_stats_url" content="{{ url_for('Airflow.dag_stats') }}">
<meta name="task_stats_url" content="{{ url_for('Airflow.task_stats') }}">
- <meta name="tree_url" content="{{ url_for('Airflow.tree') }}">
+ <meta name="tree_url" content="{{ url_for('Airflow.legacy_tree') }}">
Review comment:
There are multiple places in the JS code where the dag_id is specified as query parameters. As I've mentioned before, I would much prefer to address it in a separate PR as I imagine that it is not obvious what would be the most correct way to pass Python-generated URL to JS code with view_arg. (And I'd like to keep changes compact, rather than having a full and hard to debug rewrite in a single PR).
Are we doing to insert some templated string like '<:dag_id>'? If yes, how can we avoid breakage if the view_arg is renamed/new view_args added for a path? Or should we avoid passing dynamic URLs over the meta tags all together and come up with something else?
Plus, right now with `legacy_` and `tree` views we will have a clear indication for places that will need to be updated (which I've started working on already in subsequent 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
[GitHub] [airflow] github-actions[bot] commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1034714168
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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
[GitHub] [airflow] bbovenzi commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r795891745
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -31,15 +31,15 @@
<meta name="paused_url" content="{{ url_for('Airflow.paused') }}">
<meta name="status_filter" content="{{ status_filter }}">
<meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}">
- <meta name="graph_url" content="{{ url_for('Airflow.graph') }}">
+ <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}">
<meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}">
<meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}">
<meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}">
<meta name="csrf_token" content="{{ csrf_token() }}">
<meta name="last_dag_runs_url" content="{{ url_for('Airflow.last_dagruns') }}">
<meta name="dag_stats_url" content="{{ url_for('Airflow.dag_stats') }}">
<meta name="task_stats_url" content="{{ url_for('Airflow.task_stats') }}">
- <meta name="tree_url" content="{{ url_for('Airflow.tree') }}">
+ <meta name="tree_url" content="{{ url_for('Airflow.legacy_tree') }}">
Review comment:
Why do we need to use the legacy urls here instead of the new ones?
--
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
[GitHub] [airflow] IKholopov commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r795280975
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -110,33 +110,33 @@ <h4 class="pull-right" style="user-select: none;-moz-user-select: auto;">
<div class="row">
<div class="col-md-10">
<ul class="nav nav-pills">
- <li><a href="{{ url_for('Airflow.tree', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}">
- <span class="material-icons" aria-hidden="true">nature</span>
- Tree
+ <li><a href="{{ url_for('Airflow.dag_grid', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}">
+ <span class="material-icons" aria-hidden="true">grid_on</span>
+ Grid
</a></li>
- <li><a href="{{ url_for('Airflow.graph', dag_id=dag.dag_id, root=root, num_runs=num_runs_arg, base_date=base_date_arg, execution_date=execution_date_arg) }}">
+ <li><a href="{{ url_for('Airflow.dag_graph', dag_id=dag.dag_id, root=root, num_runs=num_runs_arg, base_date=base_date_arg, execution_date=execution_date_arg) }}">
<span class="material-icons" aria-hidden="true">account_tree</span>
Graph</a></li>
- <li><a href="{{ url_for('Airflow.calendar', dag_id=dag.dag_id) }}">
+ <li><a href="{{ url_for('Airflow.dag_calendar', dag_id=dag.dag_id) }}">
Review comment:
Done
--
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
[GitHub] [airflow] ryanahamilton commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r796972181
##########
File path: airflow/www/views.py
##########
@@ -2788,11 +2895,22 @@ def tries(self, session=None):
]
)
@action_logging
+ def legacy_landing_times(self):
+ """Redirect from url param."""
+ return redirect(url_for('Airflow.landing_times', **request.args))
+
+ @expose('/dags/<string:dag_id>/landing_times')
Review comment:
While we're changing this, could we use a hyphen instead? Hyphens are a more common, modern practice and we've used them in recent additions (e.g. `/rendered-k8s`, `/rendered-templates`).
```suggestion
@expose('/dags/<string:dag_id>/landing-times')
```
--
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
[GitHub] [airflow] bbovenzi commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r796112819
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -31,15 +31,15 @@
<meta name="paused_url" content="{{ url_for('Airflow.paused') }}">
<meta name="status_filter" content="{{ status_filter }}">
<meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}">
- <meta name="graph_url" content="{{ url_for('Airflow.graph') }}">
+ <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}">
<meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}">
<meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}">
<meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}">
<meta name="csrf_token" content="{{ csrf_token() }}">
<meta name="last_dag_runs_url" content="{{ url_for('Airflow.last_dagruns') }}">
<meta name="dag_stats_url" content="{{ url_for('Airflow.dag_stats') }}">
<meta name="task_stats_url" content="{{ url_for('Airflow.task_stats') }}">
- <meta name="tree_url" content="{{ url_for('Airflow.tree') }}">
+ <meta name="tree_url" content="{{ url_for('Airflow.legacy_tree') }}">
Review comment:
Ok thats fine. I don't want to expand the scope of this PR too much.
--
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
[GitHub] [airflow] IKholopov commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1027333474
Fixed static checks (sorry, forgot about them:) and addressed @ryanahamilton comment.
@eladkal DAG's `default_view` property used to dynamically construct URL in one of two ways:
a) `{{ url_for('Airflow.' + dag.default_view, dag_id=...)}}` - will direct to new views right away (except for `tree`, in this case it will redirect to `grid`).
b) `{{ url_for('Airflow.' + dag.default_view)}}` - changed to `{{ url_for('Airflow.legacy_' + dag.default_view)}}`, will turn into a link redirecting to new versions of views.
This configuration is one of the next places in the code to cleanup and migrate to new naming (grid). We will have to keep backwards compatibility for `tree` though.
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r795384962
##########
File path: airflow/www/decorators.py
##########
@@ -34,6 +34,15 @@
logger = logging.getLogger(__name__)
+def _get_dag_id() -> str:
+ dag_id = request.args.get('dag_id')
+ if dag_id:
+ return dag_id
+ path_parts = request.path.split('/')
+ if len(path_parts) >= 3 and path_parts[1] == 'dags':
+ return path_parts[2]
Review comment:
Instead of relying on the `/dags/<dag_id>` structure, it is likely better to use [`request.endpoint`](https://flask.palletsprojects.com/en/1.1.x/api/?highlight=request#flask.Request.endpoint) to get the `dag_id` component in the path instead.
--
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
[GitHub] [airflow] IKholopov commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1025264754
1. Renamed. This did add an additional backwards compatibility complexity. There is a number of places in code, where it is assumed that dag_id is passed exclusively over query parameter. To keep those working (especially the ones that are controlled by configuration, like [default_view](https://github.com/apache/airflow/blob/3ae4b3c0eb0d92ecead3793d80fde222329d66b4/airflow/www/templates/airflow/dag.html#L185). To keep those working, both 'tree' and 'legacy_tree' views had to be kept/introduced. I'll address those places and make them work nicely with new routing schema shortly in separate PR.
2. Done
3. Yes, I'll tackle that in subsequent PR.
--
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
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1006833716
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r780877028
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -110,33 +110,33 @@ <h4 class="pull-right" style="user-select: none;-moz-user-select: auto;">
<div class="row">
<div class="col-md-10">
<ul class="nav nav-pills">
- <li><a href="{{ url_for('Airflow.tree', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}">
- <span class="material-icons" aria-hidden="true">nature</span>
- Tree
+ <li><a href="{{ url_for('Airflow.dag_grid', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}">
+ <span class="material-icons" aria-hidden="true">grid_on</span>
+ Grid
</a></li>
- <li><a href="{{ url_for('Airflow.graph', dag_id=dag.dag_id, root=root, num_runs=num_runs_arg, base_date=base_date_arg, execution_date=execution_date_arg) }}">
+ <li><a href="{{ url_for('Airflow.dag_graph', dag_id=dag.dag_id, root=root, num_runs=num_runs_arg, base_date=base_date_arg, execution_date=execution_date_arg) }}">
<span class="material-icons" aria-hidden="true">account_tree</span>
Graph</a></li>
- <li><a href="{{ url_for('Airflow.calendar', dag_id=dag.dag_id) }}">
+ <li><a href="{{ url_for('Airflow.dag_calendar', dag_id=dag.dag_id) }}">
Review comment:
Instead of changing all these names, I’d just change the _old_ views to something like `legacy_calendar` and name the new view `calendar`. (But `tree` becomes `legacy_tree` and the new view `grid`).
--
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
[GitHub] [airflow] IKholopov commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
IKholopov commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r796105210
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -31,15 +31,15 @@
<meta name="paused_url" content="{{ url_for('Airflow.paused') }}">
<meta name="status_filter" content="{{ status_filter }}">
<meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}">
- <meta name="graph_url" content="{{ url_for('Airflow.graph') }}">
+ <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}">
<meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}">
<meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}">
<meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}">
<meta name="csrf_token" content="{{ csrf_token() }}">
<meta name="last_dag_runs_url" content="{{ url_for('Airflow.last_dagruns') }}">
<meta name="dag_stats_url" content="{{ url_for('Airflow.dag_stats') }}">
<meta name="task_stats_url" content="{{ url_for('Airflow.task_stats') }}">
- <meta name="tree_url" content="{{ url_for('Airflow.tree') }}">
+ <meta name="tree_url" content="{{ url_for('Airflow.legacy_tree') }}">
Review comment:
Yes. There are multiple places in the JS code where the dag_id is specified as a query parameter. As I've mentioned before, I would much prefer to address it in a separate PR as I imagine that it is not obvious what would be the most correct way to pass Python-generated URL to JS code with view_arg. (And I'd like to keep changes compact, rather than having a full and hard to debug rewrite in a single PR).
Are we doing to insert some templated string like '<:dag_id>'? If yes, how can we avoid breakage if the view_arg is renamed/new view_args added for a path? Or should we avoid passing dynamic URLs over the meta tags all together and come up with something else?
Plus, right now with `legacy_` and `tree` views we will have a clear indication for places that will need to be updated (which I've started working on already in subsequent 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
[GitHub] [airflow] uranusjr commented on pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1026495209
Need to fix the static checks, but logic-wise lgtm.
--
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
[GitHub] [airflow] bbovenzi commented on a change in pull request #20730: Webserver - Change URL routes for DAG page and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#discussion_r795891745
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -31,15 +31,15 @@
<meta name="paused_url" content="{{ url_for('Airflow.paused') }}">
<meta name="status_filter" content="{{ status_filter }}">
<meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}">
- <meta name="graph_url" content="{{ url_for('Airflow.graph') }}">
+ <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}">
<meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}">
<meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}">
<meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}">
<meta name="csrf_token" content="{{ csrf_token() }}">
<meta name="last_dag_runs_url" content="{{ url_for('Airflow.last_dagruns') }}">
<meta name="dag_stats_url" content="{{ url_for('Airflow.dag_stats') }}">
<meta name="task_stats_url" content="{{ url_for('Airflow.task_stats') }}">
- <meta name="tree_url" content="{{ url_for('Airflow.tree') }}">
+ <meta name="tree_url" content="{{ url_for('Airflow.legacy_tree') }}">
Review comment:
Why do we need to use the legacy urls here? Is it because of how it gets `dag_id`? I feel like it would be best to update those if it isn't too much effort.
--
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
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #20730: Modernize DAG-related URL routes and rename "tree" to "grid"
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #20730:
URL: https://github.com/apache/airflow/pull/20730#issuecomment-1034714586
Awesome work, congrats on your first merged pull request!
--
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