You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/05 00:02:48 UTC
[GitHub] [airflow] edithturn opened a new pull request #22008: Replace build image bash script in CI with build image in python
edithturn opened a new pull request #22008:
URL: https://github.com/apache/airflow/pull/22008
The script to build image is already created in python: build_image.py.
This PR replaces ./dev/breeze/src/airflow_breeze/ci/ci_build_ci_image_on_ci.sh with build_image.py
Closes #20741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062937241
Thanks, @potiuk for all your support. π€ΈπΌββοΈ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062080777
I will try to do it : `git push edithturn reuse-breeze-CI-image-building-for-CI:main`
I forgot to remove the --verbose from `Breeze2 build-ci-image --verbose` πΆβπ«οΈ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061722711
It does look good :) . Cool Push your changes here and I am ready to merge this one.
> According to what I understood, the parameters that are being sent to build-ci-image are the ones necessary for the image to be built. There are others that are optional but we are not considering them yet.
Correct. Only add those that are needed for Build 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062334206
@potiuk build image worked but something failed in tests: https://github.com/edithturn/airflow/actions , I pushed my branch into my 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062158126
@potiuk 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
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1059662372
@potiuk these are what I did and the status of this PR.
* There were many things that were synchronized with the PR of @Bowrna ππΌ
* In Breeze.py I am taking two-variable environments: **GITHUB_REPOSITORY** and **PYTHON_MAJOR_MINOR_VERSION**.
* Replacing ./scripts/ci/images/ci_build_ci_image_on_ci.sh for Breeze2 build-ci-image --verbose (verbose just for revision)
* With these changes, BUILD IMAGE works, but the problem is with Build PROD image 3.7, **which always takes more time and fails. How I can test with this, look like time out.** You can see it here: https://github.com/edithturn/airflow/runs/5430496264?check_suite_focus=true
Note: There was a random error when the image was created and then pushed, it was failing the compilation for python 3.7, 3.8 y 3.9 (randomly), the Job to Push was asking permissions to write on the package after this was built, which means the packages on my repository weren't able to overwrite. I changed the configuration of "packages" giving "write access" to each package version, it is how start to work on my own repository. You can see it here: https://github.com/potiuk/airflow/actions/runs/1919547974
I checked that the ci.yaml also has the ci_build_ci_image_on_ci.sh, should we also replace that too?
https://github.com/apache/airflow/blob/f5f221605632ebb938907bb6586537c81baa066e/.github/workflows/ci.yml#L1403-L1404
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062045759
@potiuk I saw **build_cache: Optional[str],** is not associated to any variable environment unless this is: build_cache_local what we mean by build_cache, it would make sense to change it to --build-cache-local which is a boolean value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn edited a comment on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn edited a comment on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062080777
I will try to do it : `git push origin reuse-breeze-CI-image-building-for-CI:main --force`
I forgot to remove the --verbose from `Breeze2 build-ci-image --verbose` πΆβπ«οΈ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062096002
Actually - that is a good point. We WANT --verbose in CI. But it should also be controlled by `VERBOSE` env - so please remove `--verbose` as flag, but add it as envvar - so that it will still be `verbose`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#discussion_r821941105
##########
File path: breeze
##########
@@ -836,7 +836,7 @@ function breeze::parse_arguments() {
echo
shift 2
;;
- ---platform)
Review comment:
Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062374150
> @potiuk build image worked but something failed in tests: https://github.com/edithturn/airflow/actions , I pushed my branch into my main. 
Those happen when you build in your local repo - happened to me as well. I will double check it tomorrow and likely merge it in the morning - to be around to check if there are any problems :)
Good stuff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061722711
It does look good :) . Cool Push your changes here and I am ready to merge this one.
> According to what I understood, the parameters that are being sent to build-ci-image are the ones necessary for the image to be built. There are others that are optional but we are not considering them yet.
Correct. Only add those that are needed for `build-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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062072298
> Can you share about how you are planning to replace the old breeze's build image with this one. @potiuk
I plan to simply run `./Breeze2 build-ci-image` command directly in the old `breeze` script. Just make people switch to it without even noticing :scream: :scream: :scream: .... eventually when we replace all functionality we will just replace the old `./breeze` script with `./Breeze2` :).
That will not yet work with autocomplete from the new `Breeze2` (still the ./breeze one will be used) but
* we still have to fix the scenario of installing of the autocomplete with the `pipx` and virtualenv
* we will have a chance to check if things are working for everyone else one-functionality-by-one-functionality without "big-bang"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062774921
:tada: :tada: :scream: :scream: :scream: :scream:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1059738822
This is really cool and almost works :)
Few things:
1) rebase please :)
2) please take a look at all the options and corresponding variables and update all the options to use the variables from the original `breeze`: you will see all the --flags and corresponding variables here: https://github.com/apache/airflow/blob/8c63a8d3cdc6c6d3911a8e7bde63f4fae1dde67c/breeze#L866
3) I think what you miss currently, is to make package 3.7 publicly available - you need to make sure that the package has the right access:
* It has to be public
* It has to have "airflow" repository with "write" access for Github actions
Some screenshots from my https://github.com/users/potiuk/packages/container/airflow%2Fmain%2Fci%2Fpython3.7/settings
Actions write access:
The package has to be public. Originally it is private but you can change visibility and once you do it, it should look like this:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061453973
@potiuk I updated the options with the environment variables declared on breeze, It looks like it is working fine, you can check it here: https://github.com/edithturn/airflow/actions/runs/1949750121 ,
I rebased. I am working in my main to make it possible to see build_image. Please could you check it here, first: https://github.com/edithturn/airflow/actions/runs/1949750121 , then I will update this branch.
According to what I understood, the parameters that are being sent to build-ci-image are the ones necessary for the image to be built. There are others that are optional but we are not considering them yet.
At the start, I was adding all the breeze variables, but realized they wouldn't be used yet. I just added envvar in the options of the parameters that are passed in the function.
* Just in case: You are admins of my repository
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn edited a comment on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn edited a comment on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062080777
I will try to do it : `git push origin reuse-breeze-CI-image-building-for-CI:main`
I forgot to remove the --verbose from `Breeze2 build-ci-image --verbose` πΆβπ«οΈ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061724059
We are getting really close to get `build-ci-image` functinality to be removed from the old Breeze @bowrna @edithturn . Once this once this one is merged, I am going to replace the old breeze's buld image with 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
[GitHub] [airflow] github-actions[bot] commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061730968
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 main 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062075148
> Also, I was thinking of having a development environment, where we can test this kind of thing before passing it to the main. Like this branches:
>
> * main
> * dev
> * features
That can be done easily with forks as we do now. We have another idea (but it is a bit more tricky to implement. The only reason we have this separation of code executed in build-image and ci is security (we do not want "casual" users to run builds using our self-hosted runners - they could do nasty stuff (like mining bitcoin using our machines). But we can make it so when PR comes from the maintainer (trusted user), we allow to use the code coming from PR in build.yml. This shoudl be possible but it is a bit tricky to implement,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062069672
π±
Also, I was thinking of having a development environment, where we can test this kind of thing before passing it to the main.
Like this branches:
- main
- dev
- features
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062048209
@potiuk could you please execute with your permissions to see if build_image is working fine with the update?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22008:
URL: https://github.com/apache/airflow/pull/22008
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn edited a comment on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
edithturn edited a comment on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1061453973
@potiuk I updated the options with the environment variables declared on breeze, It looks like it is working fine, you can check it here: https://github.com/edithturn/airflow/actions/runs/1949750121 ,
I rebased. I am working in my main to make it possible to see build_image, then I will update this current branch.
According to what I understood, the parameters that are being sent to build-ci-image are the ones necessary for the image to be built. There are others that are optional but we are not considering them yet.
At the start, I was adding all the breeze variables, but realized they wouldn't be used yet. I just added envvar in the options of the parameters that are passed in the function.
* Just in case: You are admins of my repository
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Bowrna commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
Bowrna commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062031217
> We are getting really close to get `build-ci-image` functinality to be removed from the old Breeze @Bowrna @edithturn . Once this once this one is merged, I am going to replace the old breeze's buld image with this one. :)
Can you share about how you are planning to replace the old breeze's build image with this one. @potiuk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062065834
> @potiuk I saw **build_cache: Optional[str],** is not associated to any variable environment unless this is: build_cache_local what we mean by build_cache, it would make sense to change it to --build-cache-local which is a boolean value.
This does not have to be set. We use `--build-cache-local` only for some very specific cases where we want to iterate over the build locally and we do not want to use remote cache. In CI we always use the default (pulled). So we can leave that 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
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1062066876
> @potiuk could you please execute with your permissions to see if build_image is working fine with the update?
I can't do it :) . It only works if it is pushed to `main` so we won't be able to see it working in `airflow` repo until we merge :scream: .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22008: Replace build image bash script in CI with build image in python
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22008:
URL: https://github.com/apache/airflow/pull/22008#issuecomment-1059739227
BTW. if you want @edithturn you can add me as admin to your fork, then I will be able to view and edit access and 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