[AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags

[zacharya19]

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-3959] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-3959
  • https://issues.apache.org/jira/browse/AIRFLOW-4026
  • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
  • In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
  • In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

• Here are some details about my PR, including screenshots of any UI changes:
  In order to allow multiple teams and users to use the same Airflow instance, I've added a tagging option for DAGs.
  Each dag can have one, a few or none tags, then the user can filter in the homepage specific tags.
  The filtering is saved in a cookie to save the user preferences.
  Before:
  

After:

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:
  test_home_filter_tags in view tests, to check if the cookie is set properly.

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

[turbaszek]

I like the idea of tagging DAGs. I found few nits and also, shouldn't we add more tests?

[zacharya19]

@nuclearpinguin Since most of this change is UI related except checking the cookie part (and come to think of it - I should separate the filter part to another function and write a small test for it, will do), not sure which tests I should add.
Any thoughts?

[OmerJog]

@zacharya19 There is an error with the test you added

ERROR: test_home_filter_tags (tests.www.test_views.TestAirflowBaseViews)
----------------------------------------------------------------------
   Traceback (most recent call last):
    tests/www/test_views.py line 329 in setUp
      super().setUp()
    tests/www/test_views.py line 70 in setUp
      self.login()
    tests/www/test_views.py line 73 in login
      role_admin = self.appbuilder.sm.find_role('Admin')
    /usr/local/lib/python3.6/site-packages/flask_appbuilder/security/sqla/manager.py line 201 in find_role
      return self.get_session.query(self.role_model).filter_by(name=name).first()
    /usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py line 3240 in first
      ret = list(self[0:1])
    /usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py line 3018 in __getitem__
      return list(res)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py line 3342 in __iter__
      return self._execute_and_instances(context)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py line 3367 in _execute_and_instances
      result = conn.execute(querycontext.statement, self._params)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py line 988 in execute
      return meth(self, multiparams, params)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/sql/elements.py line 287 in _execute_on_connection
      return connection._execute_clauseelement(self, multiparams, params)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py line 1107 in _execute_clauseelement
      distilled_params,
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py line 1253 in _execute_context
      e, statement, parameters, cursor, context
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py line 1473 in _handle_dbapi_exception
      util.raise_from_cause(sqlalchemy_exception, exc_info)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/util/compat.py line 398 in raise_from_cause
      reraise(type(exception), exception, tb=exc_tb, cause=cause)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/util/compat.py line 152 in reraise
      raise value.with_traceback(tb)
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py line 1249 in _execute_context
      cursor, statement, parameters, context
    /usr/local/lib/python3.6/site-packages/sqlalchemy/engine/default.py line 580 in do_execute
      cursor.execute(statement, parameters)
   InternalError: (psycopg2.InternalError) current transaction is aborted, commands ignored until end of transaction block
   
   [SQL: SELECT ab_role.id AS ab_role_id, ab_role.name AS ab_role_name 
   FROM ab_role 
   WHERE ab_role.name = %(name_1)s 
    LIMIT %(param_1)s]
   [parameters: {'name_1': 'Admin', 'param_1': 1}]
   (Background on this error at: http://sqlalche.me/e/2j85)

[zacharya19]

@OmerJog I don't get those errors locally, weird, checking this.

[feluelle]

@zacharya19 This seems to be related to https://issues.apache.org/jira/browse/AIRFLOW-4026. WDYT? Should we display these tags as labels next to the dag_id ?

[zacharya19]

@feluelle Yes, look like it's also related to this task.
How do I link this PR to two issues? [AIRFLOW-3959] [AIRFLOW-4026] add UI filter with DAGs tags?

I can add display the tags next to the dag_id, but what will happen if a dag has a lot of labels?
I guess it's an edge case that will not really happen, I will add it.

[feluelle]

How do I link this PR to two issues? [AIRFLOW-3959] [AIRFLOW-4026] add UI filter with DAGs tags?

Since you also added consistency for tags (in the db) I would mention that or remove the UI part of it. Like: [AIRFLOW-3959][AIRFLOW-4026] Add filter by DAG tags

I can add display the tags next to the dag_id, but what will happen if a dag has a lot of labels?
I guess it's an edge case that will not really happen, I will add it.

It could happen and I think it should be handled. I suggest to just wrap the line?

[feluelle]

Note that we also need to update the commit message to include both tickets so that both Jira tickets get linked to this PR. But it can also be done by the committer who merges it if the PR is already ready to be merged. But in your case I see you need to do some fixes first ;)

[kaxil]

Do you have any reason this great feature is limited to RBAC enabled airflow in 1.10.9? I just want to use this feature, but we don't use RBAC in our airflow.

Hi @taxpon , as mentioned in http://airflow.apache.org/blog/airflow-1.10.8-1.10.9/#use-airflow-rbac-ui We are deprecating the old Flask-based UI and will be completely removed in the next major release. We backport all the security updates to the old UI but all the new features are only applied to RBAC UI. We had already removed the old flask-based from the Master for a long time now.

[JeffryMAC]

@zacharya19 is there a way to control over the colours of the tags?

[xuejunteo]

Is there a way to auto generate permissions based on tags?

e.g.
I want to automatically grant access to user based on his role to a specific list of tags

currently only the can dag read on <dag name> and can dag write on <dag name> is auto populated when there is a new dag

it would be good if i can do something like can dag read on tag <tag name> and can dag write on tag <tag name>

thanks in advance!

[zacharya19]

@JeffryMAC The colors right now are hardcoded, what will be the motivation of controlling the color? if you want, it will be possible to add a config param to change the color, like we do in the menu bar color for example.

@xuejunteo That actually sounds like a great feature, I suggest you open a github issue and hopefully someone will pick it up (I'll try to get to it if I've time :)).