You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/04/23 21:08:25 UTC
[GitHub] [airflow] potiuk opened a new pull request, #30825: Optimize docs building in CI
potiuk opened a new pull request, #30825:
URL: https://github.com/apache/airflow/pull/30825
Docs building is the longest build for regular PRs - it takes 30 minutes for any PR that touches any of the docs or python files.
This PR optimises it - only the affected packages will be built when the PR touches only some of the files.
<!--
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 #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #30825:
URL: https://github.com/apache/airflow/pull/30825#issuecomment-1519169727
This is - I think - the last from the optimizations of CI builds. It should bring the docs building CI job down for vast majority of the PRs - from 30 to few minutes for contributors and 13 minutes for commiters to few minutes (depending on the scope of the change).
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174667421
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
I also added another test case - speecifically for common-sql, to show what happens when any of the common/sql python files change (this case is also handled - because changes in any of the python files in the dependent provider might impact generated documentation due to auto-api generated documentation from docstrings.
But the change in selective_checks I implemented is nicely handking this case too
```
pytest.param(
("airflow/providers/common/sql/common_sql_python.py",),
{
"docs-filter": "--package-filter apache-airflow "
"--package-filter apache-airflow-providers-amazon "
"--package-filter apache-airflow-providers-apache-drill "
"--package-filter apache-airflow-providers-apache-druid "
"--package-filter apache-airflow-providers-apache-hive "
"--package-filter apache-airflow-providers-apache-impala "
"--package-filter apache-airflow-providers-apache-pinot "
"--package-filter apache-airflow-providers-common-sql "
"--package-filter apache-airflow-providers-databricks "
"--package-filter apache-airflow-providers-elasticsearch "
"--package-filter apache-airflow-providers-exasol "
"--package-filter apache-airflow-providers-google "
"--package-filter apache-airflow-providers-jdbc "
"--package-filter apache-airflow-providers-microsoft-mssql "
"--package-filter apache-airflow-providers-mysql "
"--package-filter apache-airflow-providers-odbc "
"--package-filter apache-airflow-providers-oracle "
"--package-filter apache-airflow-providers-postgres "
"--package-filter apache-airflow-providers-presto "
"--package-filter apache-airflow-providers-qubole "
"--package-filter apache-airflow-providers-slack "
"--package-filter apache-airflow-providers-snowflake "
"--package-filter apache-airflow-providers-sqlite "
"--package-filter apache-airflow-providers-trino "
"--package-filter apache-airflow-providers-vertica",
},
id="Common SQL provider package python files changed changed",
),
```
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174667421
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
I also added another test case - specifically for common-sql, to show what happens when any of the common/sql python files change (this case is also handled - because changes in any of the python files in the dependent provider might impact generated documentation due to auto-api generated documentation from docstrings.
But the change in selective_checks I implemented is nicely handking this case too
```
pytest.param(
("airflow/providers/common/sql/common_sql_python.py",),
{
"docs-filter": "--package-filter apache-airflow "
"--package-filter apache-airflow-providers-amazon "
"--package-filter apache-airflow-providers-apache-drill "
"--package-filter apache-airflow-providers-apache-druid "
"--package-filter apache-airflow-providers-apache-hive "
"--package-filter apache-airflow-providers-apache-impala "
"--package-filter apache-airflow-providers-apache-pinot "
"--package-filter apache-airflow-providers-common-sql "
"--package-filter apache-airflow-providers-databricks "
"--package-filter apache-airflow-providers-elasticsearch "
"--package-filter apache-airflow-providers-exasol "
"--package-filter apache-airflow-providers-google "
"--package-filter apache-airflow-providers-jdbc "
"--package-filter apache-airflow-providers-microsoft-mssql "
"--package-filter apache-airflow-providers-mysql "
"--package-filter apache-airflow-providers-odbc "
"--package-filter apache-airflow-providers-oracle "
"--package-filter apache-airflow-providers-postgres "
"--package-filter apache-airflow-providers-presto "
"--package-filter apache-airflow-providers-qubole "
"--package-filter apache-airflow-providers-slack "
"--package-filter apache-airflow-providers-snowflake "
"--package-filter apache-airflow-providers-sqlite "
"--package-filter apache-airflow-providers-trino "
"--package-filter apache-airflow-providers-vertica",
},
id="Common SQL provider package python files changed",
),
```
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174665422
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
We are using automatically generated "generated/provider-dependencies.json" to make sure that for example when any of the google docs are changed, any provider that depends on it is also added to built docs packages
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174665239
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
Yes. That is included. See the tests on google provider.
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174665823
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
I've added a fixup to SELECTIVE_CHECKS.rst to make sure it is crystal-clear in the docs.
--
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 #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #30825:
URL: https://github.com/apache/airflow/pull/30825
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174669450
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
BTW. The whole PR here is based on the observation, that there is no way provider documentation depends on a different provider, if the provider itsefl does not depend on it (i.e. imports the other provider) - we already have that information and we are making good use of it.
Of course you could like make a reference from provider A to B is A does not depend on B, but that would be rather stupid to do and we would have found it in the "canary" build which always runs "full" build regardless.
There is an updated tests that makes sure this is happening too (`docs-filter: ""` means that all docs packages are built):
```
pytest.param(
("INTHEWILD.md",),
(),
"main",
{
"all-python-versions": "['3.7', '3.8', '3.9', '3.10']",
"all-python-versions-list-as-string": "3.7 3.8 3.9 3.10",
"image-build": "true",
"needs-helm-tests": "true",
"run-tests": "true",
"docs-build": "true",
"docs-filter": "",
"upgrade-to-newer-dependencies": "true",
"parallel-test-types": "Core Providers[-amazon,google] Other Providers[amazon] WWW "
"API Always CLI Providers[google]",
},
id="All tests run on push even if unimportant file changed",
```
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174667421
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
I also added another test case - speecifically for common-sql, to show up what happens when any of the common/sql python files change (this case is also handled - because changes in any of the python files in the dependent provider might impact generated documentation due to auto-api generated documentation from docstrings.
But the change in selective_checks I implemented is nicely handking this case too
```
pytest.param(
("airflow/providers/common/sql/common_sql_python.py",),
{
"docs-filter": "--package-filter apache-airflow "
"--package-filter apache-airflow-providers-amazon "
"--package-filter apache-airflow-providers-apache-drill "
"--package-filter apache-airflow-providers-apache-druid "
"--package-filter apache-airflow-providers-apache-hive "
"--package-filter apache-airflow-providers-apache-impala "
"--package-filter apache-airflow-providers-apache-pinot "
"--package-filter apache-airflow-providers-common-sql "
"--package-filter apache-airflow-providers-databricks "
"--package-filter apache-airflow-providers-elasticsearch "
"--package-filter apache-airflow-providers-exasol "
"--package-filter apache-airflow-providers-google "
"--package-filter apache-airflow-providers-jdbc "
"--package-filter apache-airflow-providers-microsoft-mssql "
"--package-filter apache-airflow-providers-mysql "
"--package-filter apache-airflow-providers-odbc "
"--package-filter apache-airflow-providers-oracle "
"--package-filter apache-airflow-providers-postgres "
"--package-filter apache-airflow-providers-presto "
"--package-filter apache-airflow-providers-qubole "
"--package-filter apache-airflow-providers-slack "
"--package-filter apache-airflow-providers-snowflake "
"--package-filter apache-airflow-providers-sqlite "
"--package-filter apache-airflow-providers-trino "
"--package-filter apache-airflow-providers-vertica",
},
id="Common SQL provider package python files changed changed",
),
```
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174667421
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
I also added another test case - speecifically for common-sql, to show what happens when any of the common/sql python files change (this case is also handled - because changes in any of the python files in the dependent provider might impact generated documentation due to auto-api generated documentation from docstrings.
But the change in selective_checks I implemented is nicely handking this case too
```
pytest.param(
("airflow/providers/common/sql/common_sql_python.py",),
{
"docs-filter": "--package-filter apache-airflow "
"--package-filter apache-airflow-providers-amazon "
"--package-filter apache-airflow-providers-apache-drill "
"--package-filter apache-airflow-providers-apache-druid "
"--package-filter apache-airflow-providers-apache-hive "
"--package-filter apache-airflow-providers-apache-impala "
"--package-filter apache-airflow-providers-apache-pinot "
"--package-filter apache-airflow-providers-common-sql "
"--package-filter apache-airflow-providers-databricks "
"--package-filter apache-airflow-providers-elasticsearch "
"--package-filter apache-airflow-providers-exasol "
"--package-filter apache-airflow-providers-google "
"--package-filter apache-airflow-providers-jdbc "
"--package-filter apache-airflow-providers-microsoft-mssql "
"--package-filter apache-airflow-providers-mysql "
"--package-filter apache-airflow-providers-odbc "
"--package-filter apache-airflow-providers-oracle "
"--package-filter apache-airflow-providers-postgres "
"--package-filter apache-airflow-providers-presto "
"--package-filter apache-airflow-providers-qubole "
"--package-filter apache-airflow-providers-slack "
"--package-filter apache-airflow-providers-snowflake "
"--package-filter apache-airflow-providers-sqlite "
"--package-filter apache-airflow-providers-trino "
"--package-filter apache-airflow-providers-vertica",
},
id="Common SQL provider package python files changed",
),
```
--
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] eladkal commented on a diff in pull request #30825: Optimize docs building in CI
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174664522
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
We may have cross refence between peoviders.
For example amazon provider uses common.sql example dags as part of doc build.
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
We may have cross refence between providers.
For example amazon provider uses common.sql example dags as part of doc build.
--
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 a diff in pull request #30825: Optimize docs building in CI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #30825:
URL: https://github.com/apache/airflow/pull/30825#discussion_r1174665486
##########
dev/breeze/SELECTIVE_CHECKS.md:
##########
@@ -103,51 +103,57 @@ The logic implements the following rules:
* if `Image building` is disabled, only basic pre-commits are enabled - no 'image-depending` pre-commits
are enabled.
* If there are some setup files changed, `upgrade to newer dependencies` is enabled.
+* If docs are build, the `docs-filter` will determine which docs packages to build. This is based on
+ several criteria: if any of the airflow core, charts, docker-stack, providers files or docs have changed,
Review Comment:
```
pytest.param(
("docs/apache-airflow-providers-google/docs.rst",),
{
"docs-filter": "--package-filter apache-airflow-providers-amazon "
"--package-filter apache-airflow-providers-apache-beam "
"--package-filter apache-airflow-providers-apache-cassandra "
"--package-filter apache-airflow-providers-cncf-kubernetes "
"--package-filter apache-airflow-providers-common-sql "
"--package-filter apache-airflow-providers-facebook "
"--package-filter apache-airflow-providers-google "
"--package-filter apache-airflow-providers-hashicorp "
"--package-filter apache-airflow-providers-microsoft-azure "
"--package-filter apache-airflow-providers-microsoft-mssql "
"--package-filter apache-airflow-providers-mysql "
"--package-filter apache-airflow-providers-oracle "
"--package-filter apache-airflow-providers-postgres "
"--package-filter apache-airflow-providers-presto "
"--package-filter apache-airflow-providers-salesforce "
"--package-filter apache-airflow-providers-sftp "
"--package-filter apache-airflow-providers-ssh "
"--package-filter apache-airflow-providers-trino",
},
id="Google provider docs changed",
),
```
--
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