jscheffl commented on code in PR #45266:
URL: https://github.com/apache/airflow/pull/45266#discussion_r1899170436
##########
.github/actions/prepare_breeze_and_image/action.yml:
##########
@@ -16,30 +16,48 @@
# under the License.
#
---
-name: 'Prepare breeze && current python image'
-description: 'Installs breeze and pulls current python image'
+name: 'Prepare breeze && current image (CI or PROD)'
+description: 'Installs breeze and recreates current python image from artifact'
inputs:
- pull-image-type:
- description: 'Which image to pull'
- default: CI
+ python:
+ description: 'Python version for image to prepare'
+ required: true
+ image-type:
+ description: 'Which image type to prepare (CI/PROD)'
+ default: "CI"
+ platform:
+ description: 'Platform for the build - linux/amd64 or linux/arm64'
+ required: true
outputs:
host-python-version:
description: Python version used in host
value: ${{ steps.breeze.outputs.host-python-version }}
runs:
using: "composite"
steps:
+ - name: "Cleanup docker"
+ run: ./scripts/ci/cleanup_docker.sh
+ shell: bash
- name: "Install Breeze"
uses: ./.github/actions/breeze
id: breeze
- - name: Login to ghcr.io
- shell: bash
- run: echo "${{ env.GITHUB_TOKEN }}" | docker login ghcr.io -u ${{
github.actor }} --password-stdin
- - name: Pull CI image ${{ env.PYTHON_MAJOR_MINOR_VERSION }}:${{
env.IMAGE_TAG }}
+ - name: "Restore CI docker image ${{ inputs.platform }}:${{ inputs.python
}}"
+ uses:
apache/infrastructure-actions/stash/restore@c94b890bbedc2fc61466d28e6bd9966bc6c6643c
+ with:
+ key: "ci-image-save-${{ inputs.platform }}-${{ inputs.python }}"
+ path: "/tmp/"
+ if: inputs.image-type == 'CI'
Review Comment:
The code is redundant to lines 54:59 - just `CI` and not `PROD` - besides
upper/lower case... why not using the parameter directly and removing the
redundancy? Does key need to be lower case?
##########
.github/actions/prepare_single_ci_image/action.yml:
##########
@@ -0,0 +1,43 @@
+# 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.
+#
+---
+name: 'Prepare single images'
+description: 'Recreates current python image from artifacts'
+inputs:
+ python:
+ description: 'Python version for image to prepare'
+ required: true
+ python-versions-list-as-string:
+ description: 'Stringified array of all Python versions to prepare -
separated by spaces.'
+ required: true
+ platform:
+ description: 'Platform for the build - linux/amd64 or linux/arm64'
+ required: true
+runs:
+ using: "composite"
+ steps:
+ - name: "Restore CI docker images ${{ inputs.platform }}:${{ inputs.python
}}"
+ uses:
apache/infrastructure-actions/stash/restore@c94b890bbedc2fc61466d28e6bd9966bc6c6643c
+ with:
+ key: "ci-image-save-${{ inputs.platform }}-${{ inputs.python }}"
+ path: "/tmp/"
+ if: contains(inputs.python-versions-list-as-string, inputs.python)
+ - name: "Load CI image ${{ inputs.platform }}:${{ inputs.python }}"
+ run: breeze ci-image load --platform "${{ inputs.platform }}" --python
"${{ inputs.python }}"
+ shell: bash
+ if: contains(inputs.python-versions-list-as-string, inputs.python)
Review Comment:
Why do we have this action actually? Is it not the same like
`prepare_breeze_and_image/action.yml` with parameter `CI`?
##########
.github/workflows/ci-image-build.yml:
##########
@@ -137,42 +120,32 @@ ${{ inputs.do-build == 'true' && inputs.image-tag || ''
}}"
- name: "Cleanup repo"
shell: bash
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm
-rf /workspace/*"
- if: inputs.do-build == 'true'
- name: "Checkout target branch"
uses: actions/checkout@v4
with:
persist-credentials: false
- - name: "Checkout target commit"
- uses: ./.github/actions/checkout_target_commit
- if: inputs.do-build == 'true'
- with:
- target-commit-sha: ${{ inputs.target-commit-sha }}
- pull-request-target: ${{ inputs.pull-request-target }}
- is-committer-build: ${{ inputs.is-committer-build }}
- name: "Cleanup docker"
run: ./scripts/ci/cleanup_docker.sh
- if: inputs.do-build == 'true'
- name: "Install Breeze"
uses: ./.github/actions/breeze
- if: inputs.do-build == 'true'
- - name: "Regenerate dependencies in case they were modified manually so
that we can build an image"
+ - name: "Restore CI docker image ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ uses:
apache/infrastructure-actions/stash/restore@c94b890bbedc2fc61466d28e6bd9966bc6c6643c
+ with:
+ key: "ci-image-save-${{ inputs.platform }}-${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ path: "/tmp/"
+ id: restore-ci-image
+ - name: "Load CI image ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ run: breeze ci-image load --platform ${{ inputs.platform }}
Review Comment:
+1
##########
.github/workflows/ci-image-build.yml:
##########
@@ -104,27 +97,17 @@ jobs:
strategy:
fail-fast: true
matrix:
- # yamllint disable-line rule:line-length
- python-version: ${{ inputs.do-build == 'true' &&
fromJSON(inputs.python-versions) || fromJSON('[""]') }}
+ python-version: ${{ fromJSON(inputs.python-versions) ||
fromJSON('[""]') }}
timeout-minutes: 110
- name: "\
-${{ inputs.do-build == 'true' && 'Build' || 'Skip building' }} \
-CI ${{ inputs.platform }} image\
-${{ matrix.python-version }}${{ inputs.do-build == 'true' && ':' || '' }}\
-${{ inputs.do-build == 'true' && inputs.image-tag || '' }}"
- # The ARM images need to be built using self-hosted runners as ARM macos
public runners
- # do not yet allow us to run docker effectively and fast.
- #
https://github.com/actions/runner-images/issues/9254#issuecomment-1917916016
- # https://github.com/abiosoft/colima/issues/970
- # https://github.com/actions/runner/issues/1456
- # See https://github.com/apache/airflow/pull/38640
+ name: "Build CI ${{ inputs.platform }} image ${{ matrix.python-version }}"
# NOTE!!!!! This has to be put in one line for runs-on to recognize the
"fromJSON" properly !!!!
# adding space before (with >) apparently turns the `runs-on` processed
line into a string "Array"
# instead of an array of strings.
# yamllint disable-line rule:line-length
- runs-on: ${{ (inputs.platform == 'linux/amd64') &&
fromJSON(inputs.runs-on-as-json-public) ||
fromJSON(inputs.runs-on-as-json-self-hosted) }}
+ runs-on: ${{ (inputs.platform == 'linuz/amd64') &&
fromJSON(inputs.runs-on-as-json-public) ||
fromJSON(inputs.runs-on-as-json-self-hosted) }}
Review Comment:
Typo? or by intend?
```suggestion
runs-on: ${{ (inputs.platform == 'linux/amd64') &&
fromJSON(inputs.runs-on-as-json-public) ||
fromJSON(inputs.runs-on-as-json-self-hosted) }}
```
##########
.github/workflows/ci-image-build.yml:
##########
@@ -104,27 +97,17 @@ jobs:
strategy:
fail-fast: true
matrix:
- # yamllint disable-line rule:line-length
- python-version: ${{ inputs.do-build == 'true' &&
fromJSON(inputs.python-versions) || fromJSON('[""]') }}
+ python-version: ${{ fromJSON(inputs.python-versions) ||
fromJSON('[""]') }}
timeout-minutes: 110
- name: "\
-${{ inputs.do-build == 'true' && 'Build' || 'Skip building' }} \
-CI ${{ inputs.platform }} image\
-${{ matrix.python-version }}${{ inputs.do-build == 'true' && ':' || '' }}\
-${{ inputs.do-build == 'true' && inputs.image-tag || '' }}"
- # The ARM images need to be built using self-hosted runners as ARM macos
public runners
- # do not yet allow us to run docker effectively and fast.
- #
https://github.com/actions/runner-images/issues/9254#issuecomment-1917916016
- # https://github.com/abiosoft/colima/issues/970
- # https://github.com/actions/runner/issues/1456
- # See https://github.com/apache/airflow/pull/38640
+ name: "Build CI ${{ inputs.platform }} image ${{ matrix.python-version }}"
# NOTE!!!!! This has to be put in one line for runs-on to recognize the
"fromJSON" properly !!!!
# adding space before (with >) apparently turns the `runs-on` processed
line into a string "Array"
# instead of an array of strings.
# yamllint disable-line rule:line-length
- runs-on: ${{ (inputs.platform == 'linux/amd64') &&
fromJSON(inputs.runs-on-as-json-public) ||
fromJSON(inputs.runs-on-as-json-self-hosted) }}
+ runs-on: ${{ (inputs.platform == 'linuz/amd64') &&
fromJSON(inputs.runs-on-as-json-public) ||
fromJSON(inputs.runs-on-as-json-self-hosted) }}
Review Comment:
Had the same in parallel :-D
##########
.github/actions/prepare_single_ci_image/action.yml:
##########
@@ -0,0 +1,43 @@
+# 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.
+#
+---
+name: 'Prepare single images'
Review Comment:
As the action is specific to "CI" - I'd propose to add it to the name to
make it clear - plus singular.
```suggestion
name: 'Prepare single CI image'
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
