You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "pierrejeambrun (via GitHub)" <gi...@apache.org> on 2023/10/20 15:37:47 UTC
[PR] Providers requirements for every python version [airflow]
pierrejeambrun opened a new pull request, #35086:
URL: https://github.com/apache/airflow/pull/35086
Allow to generate provider requirements for all python version. Use every python version compatible with the `associated_airflow_version` of the provider.
Parallelize image building for different python version (image with all the airflow versions pre-installed)
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367922210
##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -221,6 +221,44 @@ def get_default_platform_machine() -> str:
CURRENT_MSSQL_VERSIONS = ["2017-latest", "2019-latest"]
DEFAULT_MSSQL_VERSION = CURRENT_MSSQL_VERSIONS[0]
+
+AIRFLOW_PYTHON_COMPATIBILITY_MATRIX = {
Review Comment:
I agree, we need to update, will do in following step once the process is well tested.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1369322850
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -236,23 +241,83 @@ def update_sbom_information(
)
+@sbom.command(name="build-all-airflow-images", help="Generate images with airflow versions pre-installed")
+@option_historical_python_version
+@option_verbose
+@option_dry_run
+@option_answer
+@option_run_in_parallel
+@option_parallelism
+@option_debug_resources
+@option_include_success_outputs
+@option_skip_cleanup
+def build_all_airflow_images(
+ python: str,
+ run_in_parallel: bool,
+ parallelism: int,
+ debug_resources: bool,
+ include_success_outputs: bool,
+ skip_cleanup: bool,
+):
+ if python is None:
+ python_versions = ALL_HISTORICAL_PYTHON_VERSIONS
+ else:
+ python_versions = [python]
+
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=DockerBuildxProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ "output": outputs[index],
+ },
+ )
+ for (index, python_version) in enumerate(python_versions)
+ ]
+ check_async_run_results(
+ results=results,
+ success="All airflow base images were built successfully",
+ outputs=outputs,
+ include_success_outputs=include_success_outputs,
+ skip_cleanup=skip_cleanup,
+ )
+ else:
+ for python_version in python_versions:
+ build_all_airflow_versions_base_image(
+ python_version=python_version,
+ confirm=False,
+ output=None,
+ )
+
+
@sbom.command(name="generate-providers-requirements", help="Generate requirements for selected provider.")
+@option_historical_python_version
@click.option(
- "--airflow-version", type=str, required=False, help="Airflow version to use to generate the requirements"
-)
-@click.option(
- "--python",
- type=BetterChoice(ALL_HISTORICAL_PYTHON_VERSIONS),
- default=DEFAULT_PYTHON_MAJOR_MINOR_VERSION,
+ "--provider-id",
+ type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
required=False,
- help="Python version to generate the requirements for",
+ help="Provider to generate the requirements for",
)
@click.option(
- "--provider-id",
- type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
+ "--provider-version",
+ type=str,
Review Comment:
Updated, thanks :)
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367733237
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Here some comments how to make the images "shareable" in ghcr.io and usable for cache builds (in case you would like to pursue it @pierrejeambrun)
1. The name should be similar to the names we are using currently for CI images - see for example https://github.com/apache/airflow/pkgs/container/airflow%2Fmain%2Fci%2Fpython3.8
For example:
`ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
2. Then the image should have the right label set to indicate, it is part of apache/airflow project:
`org.opencontainers.image.source="https://github.com/apache/airflow"`
See https://github.com/apache/airflow/blob/dc2e8522bfb377daf6e2e9cf19b265f13dfe41c1/Dockerfile.ci#L1546
This will make the image "belong" to apache/airflow project in GitHub.
3. Once the image is built with the right name and label, they should be pushed from local to ghcr.io - they should appear there (this requires to have "write package" capable token - generated via github developer settings and running `docker login ghcr.io` with it.
`docker push ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
Those are just 6 images generally, so we can push them individually.
4. The person who pushed the image (owner) should go to the package settings (bottom right in the package details page of package details info found here: https://github.com/orgs/apache/packages?repo_name=airflow and:
a) change visibility to public if it is private
b) mark it as `Inherit access from source repository (recommended)` in case it is not set
5. Finally, the `docker build` command below should be changed to `docker buildx build --cache-from {image-name}` - this will use layers of the remote image in ghcr.io and rather than building them locally - which takes a lot of time and network, will pull the layers instead - which only uses the network.
I think once we do it - any of us will be able to get the "all-airflow" images quite a bit faster (providing fast network).
Let me know @pierrejeambrun if you would like to follow it up this way :) (happy to do it myself 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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367512638
##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -221,6 +221,44 @@ def get_default_platform_machine() -> str:
CURRENT_MSSQL_VERSIONS = ["2017-latest", "2019-latest"]
DEFAULT_MSSQL_VERSION = CURRENT_MSSQL_VERSIONS[0]
+
+AIRFLOW_PYTHON_COMPATIBILITY_MATRIX = {
Review Comment:
(BTW. Just testing 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367512121
##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -221,6 +221,44 @@ def get_default_platform_machine() -> str:
CURRENT_MSSQL_VERSIONS = ["2017-latest", "2019-latest"]
DEFAULT_MSSQL_VERSION = CURRENT_MSSQL_VERSIONS[0]
+
+AIRFLOW_PYTHON_COMPATIBILITY_MATRIX = {
Review Comment:
Yeah. But I think we should add it as next step. We need to update a few steps in the release process after we complete all the SBOM generation and verify it:
* updating relased airflow version here (that should be likely even via pre-commit when new version is updated)
* regenerating SBOMS for Airflow
* regenerating SBOMS for images
* regenerating package metadata when providers are released
* regenerating SBOM for providers when released.
I think adding it now is a bit premature until we test it all :) - it might cause more harm and friction in the release process
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367733237
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Here some comments how to make the images "shareable" in ghcr.io and usable for cache builds (in case you would like to pursue it @pierrejeambrun)
1. The name should be similar to the names we are using currently for CI images - see for example https://github.com/apache/airflow/pkgs/container/airflow%2Fmain%2Fci%2Fpython3.8
For example:
`ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
2. Then the image should have the right label set to indicate, it is part of apache/airflow project:
`org.opencontainers.image.source="https://github.com/apache/airflow"`
See https://github.com/apache/airflow/blob/dc2e8522bfb377daf6e2e9cf19b265f13dfe41c1/Dockerfile.ci#L1546
This will make the image "belong" to apache/airflow project in GitHub.
3. Once the image is built with the right name and label, they should be pushed from local to ghcr.io - they should appear there (this requires to have "write package" cabapble token generate via github developer settings and running `docker login ghcr.io` with it.
`docker push ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
Those are just 6 images generally, so we can push them individually.
4. The person who pushed the image (owner) should go to the package settings (bottom right in the package details page of package details info found here: https://github.com/orgs/apache/packages?repo_name=airflow and:
a) change visibility to public if it is private
b) mark it as `Inherit access from source repository (recommended)` in case it is not set
5. Finally, the `docker build` command below should be changed to `docker buildx build --cache-from {image-name}` - this will use layers of the remote image in ghcr.io and rather than building them locally - which takes a lot of time and network, will pull the layers instead - which only uses the network.
I think once we do it - any of us will be able to get the "all-airflow" images quite a bit faster (providing fast network).
Let me know @pierrejeambrun if you would like to follow it up this way :) (happy to do it myself 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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #35086:
URL: https://github.com/apache/airflow/pull/35086
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367730404
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
This way we could separately build either the image for "all versions" (if we need to regenerate sbom information for historical versions of providers for whatever reason separately.
And then - we could choose the "all-airflow" images instead of the "last" airflow" image for that.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367949177
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Thank you for the details. I would like to do that as a follow up. I will just mark the conversion resolve the conversation to not make the review harder for other, but I saved the comment link to come back to it right after this one is merged.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1774201272
Made a few adjustment to allow for more flexibility in the provider_ids/provider_versions combo. To handle:
- All historical version for providers
- latest version (provider wave release)
- specific provider_id and provider_version for adhoc things
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367733237
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Here some comments how to make the images "shareable" in ghcr.io and usable for cache builds (in case you would like to pursue it @pierrejeambrun)
1. The name should be similar to the names we are using currently for CI images - see for example https://github.com/apache/airflow/pkgs/container/airflow%2Fmain%2Fci%2Fpython3.8
For example:
`ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
2. Then the image should have the right label set to indicate, it is part of apache/airflow project:
`org.opencontainers.image.source="https://github.com/apache/airflow"
See https://github.com/apache/airflow/blob/dc2e8522bfb377daf6e2e9cf19b265f13dfe41c1/Dockerfile.ci#L1546
This will make the image "belong" to apache/airflow project in GitHub.
3. Once the image is built with the right name and label, they should be pushed from local to ghcr.io - they should appear there (this requires to have "write package" cabapble token generate via github developer settings and running `docker login ghcr.io` with it.
`docker push ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
Those are just 6 images generally, so we can push them individually.
4. The person who pushed the image (owner) should go to the package settings (bottom right in the package details page of package details info found here: https://github.com/orgs/apache/packages?repo_name=airflow and:
a) change visibility to public if it is private
b) mark it as `Inherit access from source repository (recommended)` in case it is not set
5. Finally, the `docker build` command below should be changed to `docker buildx build --cache-from {image-name}` - this will use layers of the remote image in ghcr.io and rather than building them locally - which takes a lot of time and network, will pull the layers instead - which only uses the network.
I think once we do it - any of us will be able to get the "all-airflow" images quite a bit faster (providing fast network).
Let me know @pierrejeambrun if you would like to follow it up this way :) (happy to do it myself 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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367733237
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Here some comments how to make the images "shareable" in ghcr.io and usable for cache builds (in case you would like to pursue it @pierrejeambrun)
1. The name should be similar to the names we are using currently for CI images - see for example https://github.com/apache/airflow/pkgs/container/airflow%2Fmain%2Fci%2Fpython3.8
For example:
`ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
2. Then the image should have the right label set to indicate, it is part of apache/airflow project:
`org.opencontainers.image.source="https://github.com/apache/airflow"
See https://github.com/apache/airflow/blob/dc2e8522bfb377daf6e2e9cf19b265f13dfe41c1/Dockerfile.ci#L1546
This will make the image "belong" to apache/airflow project in GitHub.
3. Once the image is built with the right name and label, they should be pushed from local to ghcr.io - they should appear there (this requires to have "write package" cabapble token generate via github developer settings and running `docker login ghcr.io` with it.
`docker push ghcr.io/apache/airflow/airflow-dev/all-airflow/python3.8:latest`
Those are just 6 images generally, so we can push them individually.
4. The person who pushed the image (owner) should go to the package settings (bottom right in the package details page of package details info found here: https://github.com/orgs/apache/packages?repo_name=airflow and:
a) change visibility to public if it is private
b) mark it as `Inherit access from source repository (recommended)` in case it is not set
5. Finally, the `docker build` command below should be changed to `docker buildx build --cache-from {image-name}` - this will use layers of the remote image in ghcr.io and rather than building them locally - which takes a lot of time and network, will pull the layers instead - which only uses the network.
I think once we do it - any of us will be able to get the "all-airflow" images quite a bit faster (providing fast network).
Let me know @pierrejeambrun if you would like to follow it up this way :)
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367948070
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
Done, we have now a separate command to build all airflow images. `breeze sbom build-all-airflow-images`.
https://github.com/apache/airflow/pull/35086/commits/05f01071c7aca09f41d3c0842a7dab4f7013dc33
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367948070
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
Done, we have now a separate command to build all airflow images. `breeze sbom build-all-airflow-images`
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1368519420
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -236,23 +241,83 @@ def update_sbom_information(
)
+@sbom.command(name="build-all-airflow-images", help="Generate images with airflow versions pre-installed")
+@option_historical_python_version
+@option_verbose
+@option_dry_run
+@option_answer
+@option_run_in_parallel
+@option_parallelism
+@option_debug_resources
+@option_include_success_outputs
+@option_skip_cleanup
+def build_all_airflow_images(
+ python: str,
+ run_in_parallel: bool,
+ parallelism: int,
+ debug_resources: bool,
+ include_success_outputs: bool,
+ skip_cleanup: bool,
+):
+ if python is None:
+ python_versions = ALL_HISTORICAL_PYTHON_VERSIONS
+ else:
+ python_versions = [python]
+
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=DockerBuildxProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ "output": outputs[index],
+ },
+ )
+ for (index, python_version) in enumerate(python_versions)
+ ]
+ check_async_run_results(
+ results=results,
+ success="All airflow base images were built successfully",
+ outputs=outputs,
+ include_success_outputs=include_success_outputs,
+ skip_cleanup=skip_cleanup,
+ )
+ else:
+ for python_version in python_versions:
+ build_all_airflow_versions_base_image(
+ python_version=python_version,
+ confirm=False,
+ output=None,
+ )
+
+
@sbom.command(name="generate-providers-requirements", help="Generate requirements for selected provider.")
+@option_historical_python_version
@click.option(
- "--airflow-version", type=str, required=False, help="Airflow version to use to generate the requirements"
-)
-@click.option(
- "--python",
- type=BetterChoice(ALL_HISTORICAL_PYTHON_VERSIONS),
- default=DEFAULT_PYTHON_MAJOR_MINOR_VERSION,
+ "--provider-id",
+ type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
required=False,
- help="Python version to generate the requirements for",
+ help="Provider to generate the requirements for",
)
@click.option(
- "--provider-id",
- type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
+ "--provider-version",
+ type=str,
Review Comment:
NIT: Maybe a comment in the description that you can use numerical version here or "latest" to only produce SBOM for latest version
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -236,23 +241,83 @@ def update_sbom_information(
)
+@sbom.command(name="build-all-airflow-images", help="Generate images with airflow versions pre-installed")
+@option_historical_python_version
+@option_verbose
+@option_dry_run
+@option_answer
+@option_run_in_parallel
+@option_parallelism
+@option_debug_resources
+@option_include_success_outputs
+@option_skip_cleanup
+def build_all_airflow_images(
+ python: str,
+ run_in_parallel: bool,
+ parallelism: int,
+ debug_resources: bool,
+ include_success_outputs: bool,
+ skip_cleanup: bool,
+):
+ if python is None:
+ python_versions = ALL_HISTORICAL_PYTHON_VERSIONS
+ else:
+ python_versions = [python]
+
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=DockerBuildxProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ "output": outputs[index],
+ },
+ )
+ for (index, python_version) in enumerate(python_versions)
+ ]
+ check_async_run_results(
+ results=results,
+ success="All airflow base images were built successfully",
+ outputs=outputs,
+ include_success_outputs=include_success_outputs,
+ skip_cleanup=skip_cleanup,
+ )
+ else:
+ for python_version in python_versions:
+ build_all_airflow_versions_base_image(
+ python_version=python_version,
+ confirm=False,
+ output=None,
+ )
+
+
@sbom.command(name="generate-providers-requirements", help="Generate requirements for selected provider.")
+@option_historical_python_version
@click.option(
- "--airflow-version", type=str, required=False, help="Airflow version to use to generate the requirements"
-)
-@click.option(
- "--python",
- type=BetterChoice(ALL_HISTORICAL_PYTHON_VERSIONS),
- default=DEFAULT_PYTHON_MAJOR_MINOR_VERSION,
+ "--provider-id",
+ type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
required=False,
- help="Python version to generate the requirements for",
+ help="Provider to generate the requirements for",
)
@click.option(
- "--provider-id",
- type=BetterChoice(list(PROVIDER_DEPENDENCIES.keys())),
+ "--provider-version",
+ type=str,
Review Comment:
NIT: Maybe a comment in the help description that you can use numerical version here or "latest" to only produce SBOM for latest version
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367466010
##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -221,6 +221,44 @@ def get_default_platform_machine() -> str:
CURRENT_MSSQL_VERSIONS = ["2017-latest", "2019-latest"]
DEFAULT_MSSQL_VERSION = CURRENT_MSSQL_VERSIONS[0]
+
+AIRFLOW_PYTHON_COMPATIBILITY_MATRIX = {
Review Comment:
I think we need to update https://github.com/apache/airflow/blob/main/dev/README_RELEASE_AIRFLOW.md by adding a new step to keep this dictionary updated after each release.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367949177
##########
dev/breeze/src/airflow_breeze/utils/cdxgen.py:
##########
@@ -217,28 +231,41 @@ def get_requirements_for_provider(
)
-def build_all_airflow_versions_base_image(python_version: str):
+def build_all_airflow_versions_base_image(
+ python_version: str,
+ confirm: bool = True,
+):
"""
Build an image with all airflow versions pre-installed in separate virtualenvs.
"""
image_name = f"apache/airflow-dev/all_airflow_versions/python{python_version}"
image_name = get_all_airflow_versions_image_name(python_version=python_version)
dockerfile = f"""
FROM ghcr.io/apache/airflow/main/ci/python{python_version}
-RUN pip install --upgrade pip
+RUN pip install --upgrade pip --no-cache-dir
# Prevent setting sources in PYTHONPATH to not interfere with virtualenvs
ENV USE_AIRFLOW_VERSION=none
+ENV START_AIRFLOW=none
"""
- for airflow_version in get_active_airflow_versions():
+ compatible_airflow_versions = [
+ airflow_version
+ for airflow_version, python_versions in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX.items()
+ if python_version in python_versions
+ ]
+
+ for airflow_version in compatible_airflow_versions:
dockerfile += f"""
# Create the virtualenv and install the proper airflow version in it
RUN python -m venv /opt/airflow/airflow-{airflow_version} && \
-/opt/airflow/airflow-{airflow_version}/bin/pip install --upgrade pip && \
+/opt/airflow/airflow-{airflow_version}/bin/pip install --no-cache-dir --upgrade pip && \
/opt/airflow/airflow-{airflow_version}/bin/pip install apache-airflow=={airflow_version} \
--constraint https://raw.githubusercontent.com/apache/airflow/\
constraints-{airflow_version}/constraints-{python_version}.txt
"""
- run_command(["docker", "build", "--tag", image_name, "-"], input=dockerfile, text=True, check=True)
+ build_command = run_command(
Review Comment:
Thank you for the details. I would like to do that as a follow up. I will just mark the conversion as `resolved` to not make the review harder for other, but I saved the comment link to get back to it right after this one is merged.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1776114632
Images need to be regenerated after my changes :(
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367923147
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -321,14 +370,14 @@ def generate_providers_requirements(
pool.apply_async(
get_requirements_for_provider,
kwds={
- "provider_id": p_id,
- "provider_version": p_version,
- "airflow_version": a_version,
- "python_version": python,
+ "provider_id": provider_id,
+ "provider_version": provider_version,
+ "airflow_version": airflow_version,
+ "python_version": python_version,
"force": force,
Review Comment:
Thanks for the tips. Done.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367725810
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=ShowLastLineProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ },
Review Comment:
Similarly here: pssing "output" from the outputs array - simialrly as in other "apply_async" cases will help to just see the progress and not the full output when parallel commands are running (providing that `build_all_airflow_versions_base_image` method will takeit as a parameter and use in "get_console()" and "run_command()" . Also in this case you probably want to use use dedicated progress matcher - that will better show the progress of the image being built.
See https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py#L143 for example (it uses DockerBuildxProgressMatcher()) - which means that it will only show the last line from docker build output showing which line is being built and it will skip any other "noise".
I hope it explains now why I havae this "parallelism" thingie.
Ut's just has all the built-in feature to make it "nice" to see the output of parallell runs - by default you see the progreess and full dump of the output if particular command fails. This can also be changed with `--include-sucess-outputs` as then it will summarize all the outputs after they finish, regardless if they succeeded or failed.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367724733
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -321,14 +370,14 @@ def generate_providers_requirements(
pool.apply_async(
get_requirements_for_provider,
kwds={
- "provider_id": p_id,
- "provider_version": p_version,
- "airflow_version": a_version,
- "python_version": python,
+ "provider_id": provider_id,
+ "provider_version": provider_version,
+ "airflow_version": airflow_version,
+ "python_version": python_version,
"force": force,
Review Comment:
I think it is better if you add "output" parameter to the "get_requirements__for_provider" and pass "output": output in here (and then use `output` in get_console() and run_command() inside the method (pass it down if needed)?
See how it is done in other cases. This is the way how the parallel outputs are nicely handled - this way outputs are not printed to stdout but they are stored in the files (separate for each output). So when commands are running you just see "progress" information happening (last line of the output).- very similar to what you see in CI when tests are running / images are build.
Then you will only see by default full output when one of those commands fail.
This will make it much easier to see what's going on.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367939699
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=ShowLastLineProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ },
Review Comment:
Not pushed yet I think :) ?
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1774989188
> Made a few adjustments to allow for more flexibility in the provider_ids/provider_versions combo.
>
> This way we can:
>
> * Regenerate requirements for all historical versions of a specific provider, (or all providers)
> * Regenerate requirements for latest versions of a specific provider (or all providers -> release wave)
> * Regenerate requirements for a specific provider and version
Whoa. Let me take a look.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1773450980
Comment 1: It takes a long time to build the images with all the airflow versions in but we can optimize it later by pushing them to ghcr.io and using them as cache.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367730805
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
Review Comment:
We should also loop through the `for (provider_id, provider_versions) in provider_metadata.items()` here - otherwisee provider_version will be None and it will always choose the latest version (we could also pass `provider_version` as parameter instead (incluing "latest") if we want to just build one provider version sbom,
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367730805
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
Review Comment:
We should also loop through the `for (provider_id, provider_versions) in provider_metadata.items()` and provider versions here - otherwise provider_version will be None and it will always choose the latest version (we could also pass `provider_version` as parameter instead (incluing "latest") if we want to just build one provider version sbom.
That is helpful for testing a subset (one provider only).
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1773876805
Thank you for the reviews.
I agree, both updating the release process and image caching are important and should be done in follow up steps.
@potiuk I will address the other comments and do the image caching as a follow up right after. Seems cool to do and I would like to give it a try, I'll let you know if I need some help :)
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367726001
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
It would be nice to have `--build-images` flag added and only run this one when the flag is set. This way we will skip even checking if the images are 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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367730181
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
Actually - probably even better if we separate it out to a separte command I think
* breeze sbom build-all-airflow-image
* breeze sbom generate-provider-requirements
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367948070
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
Review Comment:
Done, we have now a separate command to build all airflow images. `breeze sbom build-all-airflow-images`.
https://github.com/apache/airflow/pull/35086/commits/05f01071c7aca09f41d3c0842a7dab4f7013dc33. (Can build image for a specific python version, or all of the historical python version by default)
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367970934
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
Review Comment:
Done.
We can now give `provider-id` and `provider-version`.
`--provider-id`:
- specific id -> 1 provider
- None -> all provider ids found in the metadata file
`--provider-version`:
- fixed version only allowed if `--provider-id` is a specific provider (not None)
- 'latest' will pull the latest version for each selected providers.
- None will use all the historical versions.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #35086:
URL: https://github.com/apache/airflow/pull/35086#discussion_r1367924567
##########
dev/breeze/src/airflow_breeze/commands/sbom_commands.py:
##########
@@ -291,25 +291,74 @@ def generate_providers_requirements(
with open(PROVIDER_METADATA_JSON_FILE_PATH) as f:
provider_metadata = json.load(f)
providers_info = [
- (p_id, p_version, info["associated_airflow_version"])
- for (p_id, p_versions) in provider_metadata.items()
- for (p_version, info) in p_versions.items()
+ (
+ provider_id,
+ provider_version,
+ python_version,
+ info["associated_airflow_version"],
+ )
+ for (provider_id, provider_versions) in provider_metadata.items()
+ for (provider_version, info) in provider_versions.items()
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[info["associated_airflow_version"]]
+ if python_version in python_versions
]
else:
if airflow_version is None:
airflow_version = get_active_airflow_versions(confirm=False)[-1]
get_console().print(f"[info]Using {airflow_version} as airflow version")
- providers_info = [(provider_id, None, airflow_version) for provider_id in provider_ids]
+ providers_info = [
+ (provider_id, None, python_version, airflow_version)
+ for provider_id in provider_ids
+ for python_version in AIRFLOW_PYTHON_COMPATIBILITY_MATRIX[airflow_version]
+ if python_version in python_versions
+ ]
- build_all_airflow_versions_base_image(python_version=python)
+ if run_in_parallel:
+ parallelism = min(parallelism, len(python_versions))
+ get_console().print(f"[info]Running {len(python_versions)} jobs in parallel")
+ with ci_group(f"Building all airflow base images for python: {python_versions}"):
+ all_params = [
+ f"Building all airflow base image for python: {python_version}"
+ for python_version in python_versions
+ ]
+ with run_with_pool(
+ parallelism=parallelism,
+ all_params=all_params,
+ debug_resources=debug_resources,
+ progress_matcher=ShowLastLineProgressMatcher(),
+ ) as (pool, outputs):
+ results = [
+ pool.apply_async(
+ build_all_airflow_versions_base_image,
+ kwds={
+ "python_version": python_version,
+ "confirm": False,
+ },
Review Comment:
Done, I updated with output parameter and used the `DockerBuildxProgressMatcher`.
--
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
Re: [PR] Providers requirements for every python version [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35086:
URL: https://github.com/apache/airflow/pull/35086#issuecomment-1773778101
All looks good I have just a few general comments (and some improvement proposals / test results will come as inline comments).
1. We will need this "all-airflow" images only once to generate all historical versions of providers. But we might find some errors/problems/need to regenerate those so we should keep it. I already have the cache built on my local machine too, so I might push them to our ghcr.io repo and pull rather than rebuild them. I can make some comments on how this can be done easily @pierrejeambrun (might be useful to learn how caching works) - or I can do it as a follow-up PR as you wish, let me know:).
2. Later when we turn it into "generate requirements for the last wave of providers only" the default should be to use the "latest release image" for those tests only - but we should add such capability as separate 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