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/01/15 12:55:58 UTC
[GitHub] [airflow] Bowrna opened a new pull request #20886: build documentation breeze
Bowrna opened a new pull request #20886:
URL: https://github.com/apache/airflow/pull/20886
Breeze2 build documentation changes.
closes: #20739
<!--
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 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 change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785418849
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
I get `Tuple[str]`, not `List[str]` when given multiple `package_filter` as breeze command
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785418849
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
I get tuple of str not `List[str]` when given multiple `package_filter` as breeze command
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r790357887
##########
File path: dev/breeze/src/airflow_breeze/global_constants.py
##########
@@ -188,3 +188,81 @@
MSSQL_HOST_PORT = "21433"
FLOWER_HOST_PORT = "25555"
REDIS_HOST_PORT = "26379"
+
+AVAILABLE_PACKAGES = [
Review comment:
Yes.
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r790356357
##########
File path: dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from typing import List
+
+from airflow_breeze.ci.build_params import BuildParams
+from airflow_breeze.utils.path_utils import get_airflow_sources_root
+from airflow_breeze.utils.run_utils import run_command
+
+NECESSARY_HOST_VOLUMES = [
+ "/.bash_aliases:/root/.bash_aliases:cached",
+ "/.bash_history:/root/.bash_history:cached",
+ "/.coveragerc:/opt/airflow/.coveragerc:cached",
+ "/.dockerignore:/opt/airflow/.dockerignore:cached",
+ "/.flake8:/opt/airflow/.flake8:cached",
+ "/.github:/opt/airflow/.github:cached",
+ "/.inputrc:/root/.inputrc:cached",
+ "/.rat-excludes:/opt/airflow/.rat-excludes:cached",
+ "/CHANGELOG.txt:/opt/airflow/CHANGELOG.txt:cached",
+ "/LICENSE:/opt/airflow/LICENSE:cached",
+ "/MANIFEST.in:/opt/airflow/MANIFEST.in:cached",
+ "/NOTICE:/opt/airflow/NOTICE:cached",
+ "/airflow:/opt/airflow/airflow:cached",
+ "/provider_packages:/opt/airflow/provider_packages:cached",
+ "/dags:/opt/airflow/dags:cached",
+ "/dev:/opt/airflow/dev:cached",
+ "/docs:/opt/airflow/docs:cached",
+ "/hooks:/opt/airflow/hooks:cached",
+ "/logs:/root/airflow/logs:cached",
+ "/pyproject.toml:/opt/airflow/pyproject.toml:cached",
+ "/pytest.ini:/opt/airflow/pytest.ini:cached",
+ "/scripts:/opt/airflow/scripts:cached",
+ "/scripts/in_container/entrypoint_ci.sh:/entrypoint:cached",
+ "/setup.cfg:/opt/airflow/setup.cfg:cached",
+ "/setup.py:/opt/airflow/setup.py:cached",
+ "/tests:/opt/airflow/tests:cached",
+ "/kubernetes_tests:/opt/airflow/kubernetes_tests:cached",
+ "/docker_tests:/opt/airflow/docker_tests:cached",
+ "/chart:/opt/airflow/chart:cached",
+ "/metastore_browser:/opt/airflow/metastore_browser:cached",
+]
+
+
+def get_extra_docker_flags(all: bool, airflow_sources: str) -> List:
+ # get_extra_docker_flags(False, str(airflow_source))
+ EXTRA_DOCKER_FLAGS = []
+ if all:
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}:/opt/airflow/:cached"])
+ else:
+ for flag in NECESSARY_HOST_VOLUMES:
+ EXTRA_DOCKER_FLAGS.extend(["-v", airflow_sources + flag])
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}/files:/files"])
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}/dist:/dist"])
+ EXTRA_DOCKER_FLAGS.extend(["--rm"])
+ EXTRA_DOCKER_FLAGS.extend(["--env-file", f"{airflow_sources}/scripts/ci/docker-compose/_docker.env"])
+ return EXTRA_DOCKER_FLAGS
+
Review comment:
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.
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r786691573
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(get_available_packages()), multiple=True)
+def build_docs(verbose: bool, docs_only: bool, spellcheck_only: bool, package_filter: Tuple[str]):
+ """
+ Builds documentation in the container
+ """
+
Review comment:
Yeah. The only thing missing here is to actually run the `./docs/build-docs.py` inside the breeze container.
--
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 #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#issuecomment-1019612440
Just remove some of the extra stuff in .gitignore and I will be happy to merge it! Great job !
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r786690878
##########
File path: dev/breeze/src/airflow_breeze/global_constants.py
##########
@@ -188,3 +191,18 @@
MSSQL_HOST_PORT = "21433"
FLOWER_HOST_PORT = "25555"
REDIS_HOST_PORT = "26379"
+
+EXCLUDE_DOCS_PACKAGE_FOLDER = [
+ 'exts',
+ 'integration-logos',
+ 'rtd-deprecation',
+ '_build',
+ '_doctrees',
+ '_inventory_cache',
+]
+
+
+def get_available_packages() -> List[str]:
Review comment:
Here, the tests could be added to return list of packages. And compare that the list size is big enough (say > 70 :) and that it has only components starting of "airflow", "helm-chart", "docker-stack". That would be a nice test.
--
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 #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #20886:
URL: https://github.com/apache/airflow/pull/20886
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785407059
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
The option here should be "multiple" https://click.palletsprojects.com/en/8.0.x/options/#multiple-options. You should be able to specify:
--package-filter docker-stat --package-filter apache-airflow --package-filter apache-airflow-provider-google.
That will turn the "package_filter" into `List[str]`
##########
File path: dev/breeze/src/airflow_breeze/global_constants.py
##########
@@ -188,3 +188,81 @@
MSSQL_HOST_PORT = "21433"
FLOWER_HOST_PORT = "25555"
REDIS_HOST_PORT = "26379"
+
+AVAILABLE_PACKAGES = [
Review comment:
This list should be generated from the directories available in `docs` (minus few folders that we know we should skip: "exts", "integration-logos", "rtd-deprecation". It will change in the future and new packages will be added so it would be great to get this list automatically (it should be fast for click as well).
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
There is a small problem with the choices though as they allow also some other values. Package-filter can take values like `apache-airflow-provider-*`. And Click has no support for "list" autocomplete with some additional options available. Though I think this is arguable if it is useful or not. I don't think I ever used it to be honest.
I am inclined to skip the 'wildcard' capability and leave the list of packages only here.
WDYT @mik-laj ? FYI: this is the wrapper around the "build_docs.py" - with much simpler interface, executing the build always inside the breeze container and autocomplete in Breeze so having fixed set of packages might make sense actually. Maybe we can use `--package` instead of `--package-filter` to indicate that those are only lists of packages and not "filters" as it is in case of `build-docs.py. I find that most of the 'casual" users would likely only use packages and autocomplete there would be **really** useful. Where we can keep `./build_docs.py` for Power users with filtering etc.
WDYT @mik-laj
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r786759608
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(get_available_packages()), multiple=True)
+def build_docs(verbose: bool, docs_only: bool, spellcheck_only: bool, package_filter: Tuple[str]):
+ """
+ Builds documentation in the container
+ """
+
Review comment:
yes I will add it @potiuk I was not very sure if I have to invoke build_docs.py directly or via docker command. Now I am clear on that part.
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r790356323
##########
File path: dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from typing import List
+
+from airflow_breeze.ci.build_params import BuildParams
+from airflow_breeze.utils.path_utils import get_airflow_sources_root
+from airflow_breeze.utils.run_utils import run_command
+
+NECESSARY_HOST_VOLUMES = [
Review comment:
I think so - because you are prefixing it later with "${airflow_sources}" so it should be fine.
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r790355760
##########
File path: .gitignore
##########
@@ -224,3 +224,6 @@ licenses/LICENSES-ui.txt
# Packaged Breeze2 on Windows
/Breeze2.exe
+
+# Generated pre_commit_id files
+/dev/breeze/src/airflow_breeze/pre_commit_ids.py
Review comment:
Thiis shared from the other commit I guess :) ?
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r790523967
##########
File path: .gitignore
##########
@@ -224,3 +224,6 @@ licenses/LICENSES-ui.txt
# Packaged Breeze2 on Windows
/Breeze2.exe
+
+# Generated pre_commit_id files
+/dev/breeze/src/airflow_breeze/pre_commit_ids.py
Review comment:
Yeah, I will fix it up :)
--
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 #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#issuecomment-1019613855
> @potiuk we have docker resource check for many breeze commands, for build-docs too. Do we have to run that docker command and then run the build-doc specific command?
Yeah. Let's run it manually when we need it. There will be few commands that are interactive where just telling the users disk space or memory is too low might be really useful as the errors when you run out the the disk or memory might be super-confuising.
> Also I tried to print the dry run docker command, as shown in attached screenshot.
> Here for docker command, the EXTRA_DOCKER_FLAGS is passed( Where the mount happens) before running the docker command(both for resource check and build-docs). We have to include that as general function that decides to mount all file / select file depending upon the flag. After that run the docker command via subprocess in python. Am i right in understanding?
Perfect understanding - this looks great!
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r789567722
##########
File path: dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from typing import List
+
+from airflow_breeze.ci.build_params import BuildParams
+from airflow_breeze.utils.path_utils import get_airflow_sources_root
+from airflow_breeze.utils.run_utils import run_command
+
+NECESSARY_HOST_VOLUMES = [
+ "/.bash_aliases:/root/.bash_aliases:cached",
+ "/.bash_history:/root/.bash_history:cached",
+ "/.coveragerc:/opt/airflow/.coveragerc:cached",
+ "/.dockerignore:/opt/airflow/.dockerignore:cached",
+ "/.flake8:/opt/airflow/.flake8:cached",
+ "/.github:/opt/airflow/.github:cached",
+ "/.inputrc:/root/.inputrc:cached",
+ "/.rat-excludes:/opt/airflow/.rat-excludes:cached",
+ "/CHANGELOG.txt:/opt/airflow/CHANGELOG.txt:cached",
+ "/LICENSE:/opt/airflow/LICENSE:cached",
+ "/MANIFEST.in:/opt/airflow/MANIFEST.in:cached",
+ "/NOTICE:/opt/airflow/NOTICE:cached",
+ "/airflow:/opt/airflow/airflow:cached",
+ "/provider_packages:/opt/airflow/provider_packages:cached",
+ "/dags:/opt/airflow/dags:cached",
+ "/dev:/opt/airflow/dev:cached",
+ "/docs:/opt/airflow/docs:cached",
+ "/hooks:/opt/airflow/hooks:cached",
+ "/logs:/root/airflow/logs:cached",
+ "/pyproject.toml:/opt/airflow/pyproject.toml:cached",
+ "/pytest.ini:/opt/airflow/pytest.ini:cached",
+ "/scripts:/opt/airflow/scripts:cached",
+ "/scripts/in_container/entrypoint_ci.sh:/entrypoint:cached",
+ "/setup.cfg:/opt/airflow/setup.cfg:cached",
+ "/setup.py:/opt/airflow/setup.py:cached",
+ "/tests:/opt/airflow/tests:cached",
+ "/kubernetes_tests:/opt/airflow/kubernetes_tests:cached",
+ "/docker_tests:/opt/airflow/docker_tests:cached",
+ "/chart:/opt/airflow/chart:cached",
+ "/metastore_browser:/opt/airflow/metastore_browser:cached",
+]
+
+
+def get_extra_docker_flags(all: bool, airflow_sources: str) -> List:
+ # get_extra_docker_flags(False, str(airflow_source))
+ EXTRA_DOCKER_FLAGS = []
+ if all:
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}:/opt/airflow/:cached"])
+ else:
+ for flag in NECESSARY_HOST_VOLUMES:
+ EXTRA_DOCKER_FLAGS.extend(["-v", airflow_sources + flag])
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}/files:/files"])
+ EXTRA_DOCKER_FLAGS.extend(["-v", f"{airflow_sources}/dist:/dist"])
+ EXTRA_DOCKER_FLAGS.extend(["--rm"])
+ EXTRA_DOCKER_FLAGS.extend(["--env-file", f"{airflow_sources}/scripts/ci/docker-compose/_docker.env"])
+ return EXTRA_DOCKER_FLAGS
+
Review comment:
@potiuk I have added the docker resource check command. Please check and share your views about it.
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r789568117
##########
File path: dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from typing import List
+
+from airflow_breeze.ci.build_params import BuildParams
+from airflow_breeze.utils.path_utils import get_airflow_sources_root
+from airflow_breeze.utils.run_utils import run_command
+
+NECESSARY_HOST_VOLUMES = [
Review comment:
Also I have added host_volumes starting with `/`. Is it the right way to do it? @potiuk
--
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] github-actions[bot] commented on pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#issuecomment-1019612450
The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Bowrna commented on pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#issuecomment-1018136675
@potiuk we have docker resource check for many breeze commands, for build-docs too. Do we have to run that docker command and then run the build-doc specific command?
Also I tried to print the dry run docker command, as shown in attached screenshot.
Here for docker command, the EXTRA_DOCKER_FLAGS is passed( Where the mount happens) before running the docker command(both for resource check and build-docs). We have to include that as general function that decides to mount all file / select file depending upon the flag. After that run the docker command via subprocess in python. Am i right in understanding?
<img width="1436" alt="Screenshot 2022-01-21 at 8 59 04 AM" src="https://user-images.githubusercontent.com/10162465/150460661-cafa780e-d812-4e2d-9ffb-3d68a96b0b5b.png">
--
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] Bowrna commented on a change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
Bowrna commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785425643
##########
File path: dev/breeze/src/airflow_breeze/global_constants.py
##########
@@ -188,3 +188,81 @@
MSSQL_HOST_PORT = "21433"
FLOWER_HOST_PORT = "25555"
REDIS_HOST_PORT = "26379"
+
+AVAILABLE_PACKAGES = [
Review comment:
@potiuk Do i have to ignore directories starting with underscore 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.
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785421348
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
Oh yeah. Was not sure if it's tuple or.list :)
--
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 change in pull request #20886: build documentation breeze
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785421348
##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
console.print(f'[blue]Backend cached_value {backend}')
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))
Review comment:
Oh yeah. Was not aurę od it's triple or.list :)
--
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