[AIRFLOW-161] New redirect route and extra links #5059

milton0825 · 2019-04-08T05:47:15Z

Co-authored-by: Max Payton [email protected]

Jira

My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-161

Description

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

With this change different operators would be able to customize the
task instance model view with extra links, those can be used to
redirect users to systems which are out of Airflow.

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

Passes flake8

codecov-io · 2019-04-11T08:09:18Z

Codecov Report

Merging #5059 into master will increase coverage by 0.03%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master    #5059      +/-   ##
==========================================
+ Coverage   76.81%   76.85%   +0.03%     
==========================================
  Files         463      463              
  Lines       29757    29816      +59     
==========================================
+ Hits        22859    22914      +55     
- Misses       6898     6902       +4

Impacted Files	Coverage Δ
airflow/contrib/operators/qubole_operator.py	`86.79% <100%> (+1.07%)`	⬆️
airflow/models/baseoperator.py	`94.03% <90.9%> (-0.11%)`	⬇️
airflow/contrib/hooks/qubole_hook.py	`55% <90.9%> (+3.06%)`	⬆️
airflow/www/views.py	`76.38% <90.9%> (+0.35%)`	⬆️
airflow/models/taskinstance.py	`92.59% <0%> (+0.17%)`	⬆️

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 86422e7...76f8885. Read the comment docs.

milton0825 · 2019-04-11T19:04:18Z

@feng-tao this is ready for another look.

milton0825 · 2019-04-11T19:10:05Z

airflow/models/baseoperator.py

De-coupled how we define the link from the BaseOperator so that we can support global extra links (links shared by all operators) in the future easier.

feng-tao · 2019-04-11T20:48:32Z

will take a look tonight

feng-tao

some small nits, we should add a doc on how to use the extra link feature. And could you post the screenshot on extra link feature as well?

feng-tao · 2019-04-12T04:35:20Z

airflow/contrib/hooks/qubole_hook.py

feng-tao · 2019-04-12T04:35:44Z

airflow/contrib/hooks/qubole_hook.py

is there a reason we put the log as local variable not a global variable?

feng-tao · 2019-04-12T04:36:45Z

airflow/contrib/hooks/qubole_hook.py

do you what exception will be hit?

Taking a closer look and I think we don't need the try...except and we can probably remove the Session.

feng-tao · 2019-04-12T04:38:08Z

airflow/models/baseoperator.py

is this compatible in both py2 and py3 ?

tests passed so I would assume so

We have the same usage in other places:

airflow/airflow/dag/base_dag.py

Line 32 in 6b38649

__metaclass__ = ABCMeta

feng-tao · 2019-04-12T04:40:17Z

also I saw you didn't modify any existing tests, why CI fails previously?

milton0825 · 2019-04-12T06:38:54Z

Thanks for the review. The CI was failing previously because it is using the wrong endpoint.

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow.

milton0825 · 2019-04-12T16:33:50Z

@feng-tao ready for another look

feng-tao

nice. lgtm

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow.

mik-laj · 2019-04-20T16:04:13Z

airflow/www/templates/airflow/dag.html

+        var markupArr = [];
+
+        $.each(extra_links, function(i, link){
+          var url = "{{ url_for('Airflow.extra_links') }}" +


You should avoid Jinja in Javascript. I will correct it in my PR.
Reference:
#4787

mik-laj · 2019-04-24T08:38:01Z

docs/howto/operator/define_extra_link.rst

@@ -0,0 +1,53 @@
+..  Licensed to the Apache Software Foundation (ASF) under one


This is not a good place to put this information. This section should contain information on how to use and not how to create operators

I feel like it make sense to put all operator related docs all under one section, e.g. docs/howto/operator so that it is easier to users to find the related information.

I think the doc has been reconstructed to put all the user phasing info.

@ashb , I think we need to define or provide clearer guideline on which doc should be put at which folder. Maybe through the season of doc initiative or an AIP.

@milton0825 It makes sense, but only all the information about the operators would be in one place. This is not true in this case. I will suggest a different structure because I am concerned that the number of guides on the use of operators will grow rapidly. In this case, the guides about creating a new operator can be easily overlooked when it is just a small sub-point in large list.

I would like these fragment in a new page that will contain information on the creation of operators. As a part of SOD 2009, I proposed the project "4. How to create a workflow" and one of its expected deliverables is to create such a page.

A page for developing custom operators that includes:

Describing mechanisms that are important when creating an operator, such as > template fields, UI color, hooks, connection, etc.

Describing the responsibility between the operator and the hook

Considerations for dealing with shared resources (such as connections and hooks)

https://cwiki.apache.org/confluence/display/AIRFLOW/Season+of+Docs+2019

As mentors of SOD project, I set a personal goal to improve the structure of the documentation. I agree that the recommendations will be useful and I will try to develop them.

I hope my explanations were helpful.

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow. (cherry-picked from commit b282aae)

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow.

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow. (cherry-picked from commit b282aae)

milton0825 force-pushed the operator-extra-links branch from eab3ac1 to 0257ba5 Compare April 8, 2019 05:58

milton0825 changed the title ~~[AIRFLOW-161] New redirect route and extra links~~ [AIRFLOW-161][WIP] New redirect route and extra links Apr 8, 2019

milton0825 force-pushed the operator-extra-links branch 4 times, most recently from 703abb0 to ba1262d Compare April 11, 2019 07:22

milton0825 changed the title ~~[AIRFLOW-161][WIP] New redirect route and extra links~~ [AIRFLOW-161] New redirect route and extra links Apr 11, 2019

milton0825 force-pushed the operator-extra-links branch 7 times, most recently from dd95410 to ed54066 Compare April 11, 2019 07:52

milton0825 changed the title ~~[AIRFLOW-161] New redirect route and extra links~~ [AIRFLOW-161][WIP] New redirect route and extra links Apr 11, 2019

milton0825 force-pushed the operator-extra-links branch from d2c616b to ebe1e05 Compare April 11, 2019 17:22

milton0825 changed the title ~~[AIRFLOW-161][WIP] New redirect route and extra links~~ [AIRFLOW-161] New redirect route and extra links Apr 11, 2019

milton0825 commented Apr 11, 2019

View reviewed changes

feng-tao reviewed Apr 12, 2019

View reviewed changes

milton0825 changed the title ~~[AIRFLOW-161] New redirect route and extra links~~ [AIRFLOW-161][WIP] New redirect route and extra links Apr 12, 2019

[AIRFLOW-161] New redirect route and extra links

76f8885

Co-authored-by: Max Payton <[email protected]> With this change different operators would be able to customize the task instance model view with extra links, those can be used to redirect users to systems which are out of Airflow.

milton0825 force-pushed the operator-extra-links branch from bc7511b to 76f8885 Compare April 12, 2019 15:20

milton0825 changed the title ~~[AIRFLOW-161][WIP] New redirect route and extra links~~ [AIRFLOW-161] New redirect route and extra links Apr 12, 2019

feng-tao approved these changes Apr 12, 2019

View reviewed changes

feng-tao merged commit b282aae into apache:master Apr 12, 2019

milton0825 deleted the operator-extra-links branch April 13, 2019 19:03

milton0825 mentioned this pull request Apr 14, 2019

[AIRFLOW-4306] Global operator extra links #5094

Merged

6 tasks

mik-laj reviewed Apr 20, 2019

View reviewed changes

mik-laj mentioned this pull request Apr 20, 2019

[AIRFLOW-3967] Extract Jinja directive from Javascript #4787

Closed

5 tasks

feng-tao mentioned this pull request Apr 23, 2019

[AIRFLOW-4396] Provide a link to external Elasticsearch logs in UI. #5164

Merged

6 tasks

mik-laj reviewed Apr 24, 2019

View reviewed changes

dstandish mentioned this pull request Aug 28, 2020

Fix slow task instance modal with extra link #10621

Closed

		@@ -0,0 +1,53 @@
		.. Licensed to the Apache Software Foundation (ASF) under one

[AIRFLOW-161] New redirect route and extra links #5059

[AIRFLOW-161] New redirect route and extra links #5059

Uh oh!

Conversation

milton0825 commented Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira

Description

Tests

Commits

Documentation

Code Quality

Uh oh!

codecov-io commented Apr 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

milton0825 commented Apr 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feng-tao commented Apr 11, 2019

Uh oh!

feng-tao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milton0825 Apr 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feng-tao commented Apr 12, 2019

Uh oh!

milton0825 commented Apr 12, 2019

Uh oh!

milton0825 commented Apr 12, 2019

Uh oh!

feng-tao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

milton0825 commented Apr 8, 2019 •

edited

Loading

codecov-io commented Apr 11, 2019 •

edited

Loading

milton0825 Apr 12, 2019 •

edited

Loading