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/03/18 13:59:54 UTC
[GitHub] [airflow] uranusjr opened a new pull request #14878: Module-wide app fixture for test_endpoints
uranusjr opened a new pull request #14878:
URL: https://github.com/apache/airflow/pull/14878
More free-ish speedup.
Can this `app` fixture be merged with the one in #14875? They have very slightly different configs (this one does not have `init_appbuilder_views`). That would be able to further save some more time.
----------------------------------------------------------------
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] uranusjr commented on pull request #14878: Module-wide app fixture for test_endpoints
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-801990143
Cool. In that case it’s probably easier for me to rebase against #14875 and include the merge here. I’ll also look around for more similar `create_app()` calls in the mean time.
--
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] ashb merged pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #14878:
URL: https://github.com/apache/airflow/pull/14878
--
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] uranusjr commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811977770
I actually compared numbers for each change in the PR and made sure all of them go down, but threw them away once I finished 😞
> tests.cli.commands.test_role_command and test_user_command are slow and wuold likely benefit massively from fixtures too
Those look like straightforward gains! I’ll do them in a separate PR (based on this one).
--
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] ashb commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811996652
> Edit: Actually I think those are already included in this one?
Whoops, so they are!
--
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] ashb commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811962565
@uranusjr Changes look okay -- do you have before and after numbers?
--
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] ashb commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811968760
Ran some tests, this PR as it stands made tests/cli/commands go from 4m30 down to 2m12.
Nice!
--
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 #14878: Module-wide app fixture for test_endpoints
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-801962353
[The Workflow run](https://github.com/apache/airflow/actions/runs/664759497) is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.
----------------------------------------------------------------
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] ashb commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811968184
Don't know if we want to do it in this PR, but tests.cli.commands.test_role_command and test_user_command are slow and wuold likely benefit massively from fixtures too.
--
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] ashb closed pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #14878:
URL: https://github.com/apache/airflow/pull/14878
--
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] uranusjr commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-802464073
I’m going with `app` for now.
--
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] uranusjr edited a comment on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-811977770
--
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] uranusjr edited a comment on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-802464073
I’m going with `app` for now. I think these are all the `create_app()` speedups I can find (aside from those in #14863 which are more problematic).
--
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] uranusjr closed pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #14878:
URL: https://github.com/apache/airflow/pull/14878
--
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] uranusjr commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-802142848
I’ve rebased this against #14875, so this now includes changes from that PR. If that gets merged first I’ll rebase/merge again.
I’ve located several more app creation code and change them to be session or class scoped. I think there are some more possibile optimisations, especially several of the apps are simply bare `create_app(testing=True)` without decorators. I think they can be merged into a session-scoped fixture in the top-level `tests/conftest.py`. What would be a good name for the fixture? `app` sounds wrong to me, maybe
```python
@pytest.fixture(scope="session")
def testing_app():
return application.create_app(testing=True)
```
?
--
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] ashb commented on pull request #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-803148129
Closing and re-opening to trigger full CI rebuild.
--
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] ashb commented on pull request #14878: Module-wide app fixture for test_endpoints
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-801984721
We possibly could merge them, yes. The reason to not call linit_appbuilder_views is that it is slow, but if we can reduce the total number of times we initialize the app it will be faster overall _if_ we run these tests in the same "batch".
We don't run `pytests tests/` but we run some in batches -tests/www, tests/api, etc.
But with a change like this maybe it doesn't make as much sense to split these apart anymore.
--
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 #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-809819168
--
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 #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-803189968
--
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 #14878: Make app creation session- or class-wide when possible to make tests faster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14878:
URL: https://github.com/apache/airflow/pull/14878#issuecomment-809818990
[The Workflow run](https://github.com/apache/airflow/actions/runs/699627723) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
--
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