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 2021/01/26 04:45:22 UTC
[GitHub] [airflow] jedcunningham opened a new pull request #13903: More flexibility with FAB menu links
jedcunningham opened a new pull request #13903:
URL: https://github.com/apache/airflow/pull/13903
Airflow can be more flexible with the links plugins are allowed to add. Currently, you cannot add a top level link, a link with a label, or even without providing a category_icon (which isn't used anyways).
This PR makes allows plugin authors the flexibility to add any link FAB supports.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#issuecomment-780211044
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#discussion_r577315218
##########
File path: tests/www/test_views.py
##########
@@ -488,7 +488,7 @@ def prepare_dagruns(self):
)
def test_index(self):
- with assert_queries_count(42):
+ with assert_queries_count(43):
Review comment:
It's the new menu link I added (based on me adding a second new link and it going up to 44). I don't know why FAB is doing a query per menu link, but that seems to be the behavior 🤷♂️.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #13903:
URL: https://github.com/apache/airflow/pull/13903
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#discussion_r577237113
##########
File path: tests/www/test_views.py
##########
@@ -488,7 +488,7 @@ def prepare_dagruns(self):
)
def test_index(self):
- with assert_queries_count(42):
+ with assert_queries_count(43):
Review comment:
Any idea what caused an extra query?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#issuecomment-778728489
Thanks for explaining @jedcunningham. I think this makes a lot of sense and is a good update.
@kaxil this code is good to go.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#issuecomment-774609207
Hi @jedcunningham! Thanks for adding this with solid looking tests. Just to make this easier to review, would it be possible to add an image of a top-level link and one with a label?
Out of curiosity, is there a specific use case you have that caused you to suggest these changes?
@ryanahamilton
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on pull request #13903: More flexibility with FAB menu links
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #13903:
URL: https://github.com/apache/airflow/pull/13903#issuecomment-775296507
@jhtimmins I recently added an extra link to Docs and found it odd that I needed to provide a category_icon, so I went and looked at FAB add_link upstream and saw that it's unused for existing categories anyways. I also noticed one couldn't create a top-level link if they wanted one, so I figured it was worth a PR.
```python
appbuilder_menu_items = [
{"name": "Hello", "category": "Docs", "href": "https://google.com",},
{
"name": "labelled",
"label": "Hello (label)",
"category": "Docs",
"href": "https://google.com",
},
{"name": "Top Hello", "href": "https://google.com",},
]
```
Leads to:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org