Skip to content

Filter dags by owner#11121

Merged
ryanahamilton merged 3 commits intoapache:masterfrom
Sangarshanan:filter-owner
Nov 9, 2020
Merged

Filter dags by owner#11121
ryanahamilton merged 3 commits intoapache:masterfrom
Sangarshanan:filter-owner

Conversation

@Sangarshanan
Copy link
Contributor

Filter DAGs by clicking on the dag owner

closes: #9102


Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Sep 24, 2020
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should not display a separate link for each person.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you think we should do this ? Also why not links, I am asking because everything else is a link 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                  {% for owner in dag.owners.split(",") %}
                  <a class="label label-default"
                     href="?search={{ owner | trim }}"
                     style="margin: 3px;">
                     {{ owner | trim }}
                  </a>
                  {% endfor %}

Screenshot 2020-11-08 at 01 08 57

What do you think aboiut it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before:
Screenshot 2020-11-08 at 01 11 24

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes I think I understood the question wrong, this does make more sense

@kaxil
Copy link
Member

kaxil commented Nov 4, 2020

Can you please rebase your PR on latest Master since we have applied Black and PyUpgrade on Master.

It will help if your squash your commits into single commit first so that there are less conflicts.

Copy link
Member

@mik-laj mik-laj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I would love to hear feedback from @ryanahamilton .

@github-actions
Copy link

github-actions bot commented Nov 8, 2020

The PR should be OK to be merged with just subset of tests as it does not modify Core of Airflow. The committers might merge it or can add a label 'full tests needed' and re-run it to run all tests if they see it is needed!

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Nov 8, 2020
Copy link
Contributor

@ryanahamilton ryanahamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minuscule styling suggestion, but otherwise LGTM!

Co-authored-by: Ryan Hamilton <ryan@ryanahamilton.com>
@ryanahamilton ryanahamilton merged commit 8f423c7 into apache:master Nov 9, 2020
@Sangarshanan Sangarshanan deleted the filter-owner branch November 9, 2020 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:webserver Webserver related Issues okay to merge It's ok to merge this PR as it does not require more tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to filter DAG list by clicking on dag owner

4 participants