potiuk commented on a change in pull request #14120:
URL: https://github.com/apache/airflow/pull/14120#discussion_r578049910
##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -272,74 +272,89 @@ function push_pull_remove_images::push_prod_images() {
fi
}
-# waits for an image to be available in GitHub Packages
-function push_pull_remove_images::wait_for_image_in_github_packages() {
+# waits for an image to be available in GitHub Packages. Should be run with
`set +e`
+# the buid automatically determines which registry to use based one the images
available
Review comment:
It's not an optimization - it's a feature (and this is the only reason
why this PR is implemented).
This "auto-detection" here is precisely for the reason we found today that
the old code of switching "wait for images" would not work. This Script is
used in ci.yml and `GITHUB_REGISTRY` cannot be set as global variable if you
want to enable and easy switch without having to update `ci.yml`
The secret to switch images is not available in fork PR workflow. Currently
the only way we can communicate the type of registry we use is by checking if
the image is available in both registries and when it is in one of them - set
`GITHUB_REGISTRY` value based on that. This is the same mechanism we already
used to see if the images are actually built - we checked if they are pushed to
the registry. Now we additionally check from which registry they came so that
we can set `GITHUB_REGISTRY` variable based on that. So the `GITHUB_REGISTRY`
This has the advantage over other solutions that when you change the
registry in secret, you do not have to update the ci.yml (which means that PRs
do not have to rebased to take advantage. Basically the ci.yml is identical, no
matter which registry is used. This was the whole point of this PR - to be able
to switch between the registries freely and without disruption to existing PRs.
Of course we do not have to do it. We can simply switch permanently - that
would be easier, but we actually cannot switch entirely to gcr.io . The problem
is that then if someone tries to build it in their own fork as PR/master push
it will not work without pre-configuraation. GHCR requires few steps to set up
(crating repos and assigning permissions) to work. As I described in CI.rst:
> The new `GitHub Container Registry
<https://docs.github.com/en/packages/guides/about-github-container-registry>`_
which is in Public Beta, has many more features (including permission
management, public access and
image retention possibility). It has also the drawback (at least as of
January 2020) that you need to
have separate personal access token created as ``PAT_CR`` secret in your
repository with write access
to registry in order to make it works. You also have to manually manage
permissions of the images,
i.e. after creating images for the first time, you need to set their
visibility to "Public" and
add ``Admin`` permissions to group of people managing the images (in our
case ``airflow-committers`` group).
This makes it not very suitable to use GitHub container registry if you
want to run builds of Airflow
in your own forks (note - it does not affect pull requests from forks to
Airflow).
So when we switch to GHCR in `apache` repo the same ci.yml should use GHCR
for the `apache` repo and the docker.package registry for all other
repositories.
Of course we could add some "ifs" in both build and ci workflows (if
github.repository == "apache/airflow" ) but using ifs to set env variables (as
you saw above) is messy - here the 'falsy' value would not help and github
expression language has no ternary operator. However it has the big drawback
that you need to synchronize ci.yml and build.yml and PRs will only work after
rebasing if we change the hard-coded values in the expression. So rather than
hardcode the value, I used auto-detection.
I would be happy if you find a simpler way of doing it - so far I could not
find an easier way to make existing PRs to take advantage of the configuration
(secret in this case) in the apache repo.
Any ideas ?
----------------------------------------------------------------
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:
[email protected]
