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 2022/12/08 22:52:01 UTC
[GitHub] [airflow] potiuk opened a new pull request, #28244: Speed up airlfow role list command
potiuk opened a new pull request, #28244:
URL: https://github.com/apache/airflow/pull/28244
The command initialized whole flask_app of ours, but what we really needed was FAB security manager only.
Fixes: #28242
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow role list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1343476035
Maybe not the "FINAL" fix but on my slow local DB:
```
time airflow roles list
```
Before:
```
name
======
Admin
Op
Public
User
Viewer
airflow roles list 5.49s user 1.79s system 136% cpu 5.339 total
```
After:
```
name
======
Admin
Op
Public
User
Viewer
airflow roles list 2.43s user 1.75s system 198% cpu 2.103 total
```
@malthe - can you check it on your system please ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344269372
I will apply it elsewhere then :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344128189
@malthe - i sped it up a bit more - I suspected I do not even need "import_all_models" - so now it should be even faster.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344111033
> Could this improvement be applied to `roles_create` as well?
Very much so. I can do another PR
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
malthe commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1343957876
This cut our runtime from 49s to 9s.
But what about `airflow roles create` ...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344117907
There are quite a few other commands like that in user/role management that we could improve this way.
Hey @ashb @kaxil @jhtimmins and others - do you think there is any drawback to this solution ?
It does not initialize quite a lot of Airlfow Webserver pieces and we get 3x - 5x speedup for those commands.
Potentially I can imagine some subtle changes that somoene might want to add in their plugins might stop working with the CLI commands if we limit initialization to only Flask App Builder. But I'd argue the CLIs of our was never meant to be extended by plugins, so I see that as "feature" not "bug".
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344120072
BTW. I think we can speed it up even more by reaping apart some of the FAB code doing the intialization - but this is the "Pareto" rule in full swing - 80% of gain in 20% of the effort.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #28244:
URL: https://github.com/apache/airflow/pull/28244#issuecomment-1344194192
LGTM.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #28244: Speed up airlfow roles list command
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #28244:
URL: https://github.com/apache/airflow/pull/28244
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org