You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/26 09:18:23 UTC

[GitHub] [airflow] potiuk opened a new pull request #15528: Adds script to automate renaming docker images from master to main

potiuk opened a new pull request #15528:
URL: https://github.com/apache/airflow/pull/15528


   <!--
   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/master/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/master/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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #15528: Updates branches and branch documentation after 2.1.0rc1

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r635268376



##########
File path: .github/workflows/ci.yml
##########
@@ -21,9 +21,9 @@ on:  # yamllint disable-line rule:truthy
   schedule:
     - cron: '28 0 * * *'
   push:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test']
   pull_request:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test', 'v*-*-stable']

Review comment:
       Yeah, this was one of those "I feel like it should be possible to me" cases and was in the mindset to go digging 😁 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-830861670


   Those are scripts to move the GitHub and DockerHub "master" images to "main"  (basically re-tagging them). Since we are going to implement the "master" -> "main" rename eventually (https://lists.apache.org/thread.html/r97f52ffa37868be325c22c48cef2e28639538c35f99eff4807931556%40%3Cdev.airflow.apache.org%3E) - having script to do it semi-automatically will be helpful. 
   
   Those images are used in Github Actions and Breeze (as cache of the latest built images) - thank to those building an image in GitHub CI takes < 1 m in most cases and refreshing Breeze image is also < 2 minutes (instead of ~ 10-15 minutes).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #15528: Updates branches and branch documentation after 2.1.0rc1

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #15528:
URL: https://github.com/apache/airflow/pull/15528


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-834635103


   Hey @ashb - small one but useful for master-> main 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-843382984


   I updated the change and also reviewed and checked all other places where v2-1 should be updated/added. There are still few places that will need to be manually updated and we can generalize them later if needed (and figure out simple way of doing 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624747760



##########
File path: IMAGES.rst
##########
@@ -320,10 +320,10 @@ Images built as "Run ID snapshot":
 
 .. code-block:: bash
 
-  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUNID>    - for CI images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUNID>       - for production images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUNID> - for production build stage
-  docker.pkg.github.com/apache-airflow/pythonX.Y-<BRANCH>-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images
+  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUN_ID>    - for CI images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUN_ID>       - for production images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUN_ID> - for production build stage
+  docker.pkg.github.com/apache-airflow/python-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images

Review comment:
       Absolutely - you are perfectly right @XD-DENG. I've added all the explanation as the commit message




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624748717



##########
File path: IMAGES.rst
##########
@@ -320,10 +320,10 @@ Images built as "Run ID snapshot":
 
 .. code-block:: bash
 
-  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUNID>    - for CI images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUNID>       - for production images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUNID> - for production build stage
-  docker.pkg.github.com/apache-airflow/pythonX.Y-<BRANCH>-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images
+  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUN_ID>    - for CI images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUN_ID>       - for production images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUN_ID> - for production build stage
+  docker.pkg.github.com/apache-airflow/python-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images

Review comment:
       And added it as comment in the script.
   
   Indeed. Assumptions (which I have and others don't) are the root of evil :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #15528: Updates branches and branch documentation after 2.1.0rc1

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-844095965


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #15528: Updates branches and branch documentation after 2.1.0rc1

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r634937195



##########
File path: .github/workflows/ci.yml
##########
@@ -21,9 +21,9 @@ on:  # yamllint disable-line rule:truthy
   schedule:
     - cron: '28 0 * * *'
   push:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test']
   pull_request:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test', 'v*-*-stable']

Review comment:
       Ah ! Cool. In the https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestbranchestags they only mention glob, but my bad I have not go to the "read more details" link :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] XD-DENG commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624742560



##########
File path: IMAGES.rst
##########
@@ -320,10 +320,10 @@ Images built as "Run ID snapshot":
 
 .. code-block:: bash
 
-  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUNID>    - for CI images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUNID>       - for production images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUNID> - for production build stage
-  docker.pkg.github.com/apache-airflow/pythonX.Y-<BRANCH>-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images
+  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUN_ID>    - for CI images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUN_ID>       - for production images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUN_ID> - for production build stage
+  docker.pkg.github.com/apache-airflow/python-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images

Review comment:
       Are these changes simply cleaning/correcting? Or they are related to the main change in this 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-843324078


   Yep. As we saw it is useful for other situations too :). Doing it now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624745678



##########
File path: IMAGES.rst
##########
@@ -320,10 +320,10 @@ Images built as "Run ID snapshot":
 
 .. code-block:: bash
 
-  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUNID>    - for CI images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUNID>       - for production images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUNID> - for production build stage
-  docker.pkg.github.com/apache-airflow/pythonX.Y-<BRANCH>-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images
+  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUN_ID>    - for CI images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUN_ID>       - for production images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUN_ID> - for production build stage
+  docker.pkg.github.com/apache-airflow/python-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images

Review comment:
       They are correcting. The names were wrong (I found it while preparing the script). Happy to split it off if needed 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r634743130



##########
File path: .github/workflows/ci.yml
##########
@@ -21,9 +21,9 @@ on:  # yamllint disable-line rule:truthy
   schedule:
     - cron: '28 0 * * *'
   push:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test']

Review comment:
       ```suggestion
       branches: ['master', 'v[0-9]+-[0-9]+-test']
   ```
   
   https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#patterns-to-match-branches-and-tags




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] XD-DENG commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624743014



##########
File path: dev/rename_docker_images_to_main.py
##########
@@ -0,0 +1,70 @@
+#!/usr/bin/python3

Review comment:
       Do we have a documentation change to record usage/purpose of this script?

##########
File path: dev/rename_docker_images_to_main.py
##########
@@ -0,0 +1,70 @@
+#!/usr/bin/python3
+#
+# 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.
+import subprocess
+from typing import List
+
+PYTHON_VERSIONS = ["3.6", "3.7", "3.8"]
+
+DOCKERHUB_PREFIX = "apache/airflow"
+
+DOCKERHUB_IMAGES = [
+    "{prefix}:python{python_version}-{branch}",

Review comment:
       Is image tag purposely ignored for `DOCKERHUB_IMAGES`?
   
   I notice they are all `latest` for `GITHUB_REGISTRY_IMAGES` and `GHCR_IO_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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r634741878



##########
File path: .github/workflows/ci.yml
##########
@@ -21,9 +21,9 @@ on:  # yamllint disable-line rule:truthy
   schedule:
     - cron: '28 0 * * *'
   push:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test']
   pull_request:
-    branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
+    branches: ['master', 'v*-*-test', 'v*-*-stable']

Review comment:
       ```suggestion
       branches: ['master', 'v[0-9]+-[0-9]+-test', 'v[0-9]+-[0-9]+-stable']
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624746604



##########
File path: dev/rename_docker_images_to_main.py
##########
@@ -0,0 +1,70 @@
+#!/usr/bin/python3
+#
+# 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.
+import subprocess
+from typing import List
+
+PYTHON_VERSIONS = ["3.6", "3.7", "3.8"]
+
+DOCKERHUB_PREFIX = "apache/airflow"
+
+DOCKERHUB_IMAGES = [
+    "{prefix}:python{python_version}-{branch}",

Review comment:
       The problem with DOCKERHUB_IMAGES is that they are all "apache/airflow" - because that's the only image we have access to in DockerHub. So in DockerHub the image is always "apache/airflow" but has a different tag. 
   
   In GitHub registries (depends on the type of registry) we have more flexibility":
   * in the old GitHub docker registry - docker.pkg.github.com -  (current but already deprecated) we can use "apache/airflow/IMAGE:tag"
   * in the new package registry (ghcr.io) - we can submit anything under apache/airflow-* but then we link it to the project via docker image label.
   
   So naming conventions for all three types of images are different :(




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-826664636


   @ashb -> I already pushed all the images with `master`-> `main` rename and I can use the script to refresh those easily. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] XD-DENG commented on a change in pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#discussion_r624746800



##########
File path: IMAGES.rst
##########
@@ -320,10 +320,10 @@ Images built as "Run ID snapshot":
 
 .. code-block:: bash
 
-  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUNID>    - for CI images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUNID>       - for production images
-  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUNID> - for production build stage
-  docker.pkg.github.com/apache-airflow/pythonX.Y-<BRANCH>-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images
+  docker.pkg.github.com.io/apache-airflow/<BRANCH>-pythonX.Y-ci-v2:<RUN_ID>    - for CI images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-v2:<RUN_ID>       - for production images
+  docker.pkg.github.com/apache-airflow/<BRANCH>-pythonX.Y-build-v2:<RUN_ID> - for production build stage
+  docker.pkg.github.com/apache-airflow/python-v2:X.Y-slim-buster-<RUN_ID>  - for base Python images

Review comment:
       Personally I'm ok to keep them here, but good to update the commit msg and PR title/description as well. Do you agree?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #15528: Adds script to automate renaming docker images from master to main

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15528:
URL: https://github.com/apache/airflow/pull/15528#issuecomment-827923793


   RE-pushed latest images. Would be nice to get it to master :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org