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 2019/01/16 00:04:37 UTC

[GitHub] ArgentFalcon opened a new pull request #3533: [AIRFLOW-161] New redirect route and extra links

ArgentFalcon opened a new pull request #3533: [AIRFLOW-161] New redirect route and extra links
URL: https://github.com/apache/airflow/pull/3533
 
 
   Make sure you have checked _all_ steps below.
   
   ### JIRA
   - [x] My PR addresses the following [Airflow JIRA](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
       - https://issues.apache.org/jira/browse/AIRFLOW-161
   
   
   ### Description
   - [X] Here are some details about my PR, including screenshots of any UI changes:
   
   This PR is an attempt to finish the work started in #2657
   
   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.
   
   In order to be able to display the link on UI, and not have the backend do the external routing, I had to setup the endpoint to
   be a RESTful endpoint that the frontend could ping whenever. I also wanted to handle a handful of error cases, so it returns different errors
   If the URL is not whitelisted, if the URL is not provided, and there is even a way for operator to provide their own custom errors
   
   
   Update the modal box to make a AJAX call for link resolutions
   This gives us multiple benefits:
    1. It enables us to disable links when actionable
    2. It enables us to give better feedback to the user on whitelisting and errors without navigating the page
    3. You can see and interact with the link as it points to it's real destination, as opposed to the airflow backend doing the routing
      This let's you see where the link will resolve in advance
   
   **Note**: A lot of the changes here are duplicating UI work between `www` and `www_rbac`
   
   New additions: 
   You can now see where the link will resolve in advance of clicking it:
   <img width="808" alt="screenshot 2018-06-21 18 33 22" src="https://user-images.githubusercontent.com/3519697/41753344-be1bcd5e-7581-11e8-8764-2bc790a655b7.png">
   
   Domains that aren't whitelisted will show a tooltip and be disabled
   <img width="627" alt="screenshot 2018-06-21 18 33 36" src="https://user-images.githubusercontent.com/3519697/41753345-be304432-7581-11e8-85cd-052b4a9df0cf.png">
   
   An operator can set a custom error message to show up on a tooltip (for example, if a link is not yet available for some reason, e.g. A job hasn't run yet, so no logs exist)
   <img width="622" alt="screenshot 2018-06-21 18 33 42" src="https://user-images.githubusercontent.com/3519697/41753346-be47819c-7581-11e8-99c6-c355ae2d694f.png">
   
   I should also mention here that at Lyft, we've had this running in production for half a year, and it's been really useful so far. 
   
   I will also make a follow up PR with some improvements we've made to extra_links, allowing the user to specify extra_links as plugins that will apply to all operators. We use this feature to link to some logging tools we use (Kibana)
   
   ### Tests
   - [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   tests/www/test_views.py::TestRedirect
   tests/www_rbac/test_views.py::TestRedirect
   tests/contrib/operators/test_qubole_operator.py::QuboleOperatorTest::test_get_redirect_url
   tests/models.py::DagTest::test_extra_links_no_affect
   tests/models.py::DagTest:: test_extra_links
   
   
   ### Commits
   - [X] 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](http://chris.beams.io/posts/git-commit/)":
       1. Subject is separated from body by a blank line
       2. Subject is limited to 50 characters
       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
   - [x] In case of new functionality, my PR adds documentation that describes how to use it.
       - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
   
   
   ### Code Quality
   - [x] Passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services