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 2020/11/20 15:55:44 UTC
[GitHub] [airflow] turbaszek opened a new pull request #12511: Run pip check when to verify production images
turbaszek opened a new pull request #12511:
URL: https://github.com/apache/airflow/pull/12511
This is first step to address #12508
---
**^ 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] potiuk edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731777833
Proposal:
In ci.yaml we have this: (added some comments):
```
constraints-push:
......
# here we have all the constrains generated in CI image in "artifacts" folder . So in case if
# "UPGRADE_TO_LATEST_CONSTRAINTS" was derived, we should get the new constraints here
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
# you can see how to retrieve the artifacts in the ./scripts/ci/constraints/ci_commit_constraints.sh below
# In order to build image with those new constraints you need to copy the constraints to "./docker-context-files" and
# set AIRFLOW_CONSTRAINTS_LOCATION="./docker-context-files/constraints-${PYTHON_MAJOR_MINOR_VERSION}"
# This will build the production image with those new constraints and then you will be ablet to run `pip check`
"Commit changed constraint files for ${{needs.build-info.outputs.pythonVersions}}"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
```
I think you will have to add a new matrix job (inject it before 'constraint push" and add the same prerequisites). Because we want to run `pip check` for all python versions. Current "constraint_push" is not a matrix job -it simply performs one commit with all the constraints for all python files that were generated in the build.
It should have likely few steps similar to those:
```
env:
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
- name: "Extract constraint files to ./docker-context-files
run: .... # Here you need to copy the constraint files from "artifacts" . You want to copy only constraint files as "artifacts" will be far too big (and it takes a lot of time to upload big context to docker engine before the build)
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
env:
- AIRFLOW_CONSTRAINTS_LOCATION: "./docker-context-files/constraints-...."
- name: "Run PIP check"
run: ...
```
Then you should make the "constraints-push" depend on the above job (and only this job). This way we will only update the constraints in "constraints-master" branch when both CI tests passes, and `pip-check` passes.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731834869
> I will take a look tomorrow. But this PR aimed to validate only prod image. And I think having this check in the same place where we run other validations for image make sense. And it does not require additional matrix = less jobs to run in parallel.
I am afraid it has to be done in the matrix and using the upgraded constraints from CI builds.
We have quite a different set of dependencies for different python versions. And if a pip check succeeds on one there is no guarantee it wil succeed with the other. We also cannot upgrade the constraints when we are building the image for regular PRs - because of our transitivie dependency problems.
If we do this, then we go back to the situation we hed where PRs start failing because someone released a new version of a transitive library we are using (remember werkzeug drama ?).
The constraint mechanism is specially designed to prevent this case. Do you remember the last time it happened recently for us ? Probably not because we are preventing this from happening and the constraint mechanism does it.
So we have to only run upgrade constraints in case of the "master pushschedule builds" and the constraints are now only really updated after all tests pass for them. And this is perfect time to do the pip check - not sooner, not later. Because this is the moment where the constraints get updated. And we must do a pip check there, otherwise pip check will start failing in master after such constraint push. So we really have to do the pip-check just before we push updated constraints.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731329208
It is likely required by another dependency transitively. We have to run "pipdeptree" to find out.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732131401
[The Workflow run](https://github.com/apache/airflow/actions/runs/378908653) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI). The github.sha is needed because we want to upgrade as soon as possible
# And not when setup.py changes only. This is because we need to invalidate the layer in docker when we first install the
# pip dependencies for caching (so that we can run the subsequent `pip install http://github'. If we use "SHA" as the value of
# upgradeToLatestConstraints, it will still upgrade them every time wen new commit is merged - otherwise Docker cache is
# used and no upgrade is done.
if push or schedule -> upgradeToLatestConstraints = github.sha
```
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-735279535
Closing as it has been merged into https://github.com/apache/airflow/pull/12664
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731775663
I generally think failing Prod image by PIP check in workflow build is a bad idea. Because we don't even see the tests running then. I think such a check should only run AFTER all the tests are run and before we Push the new constraints. That is also better, because we can then make sure that we are doing the check in the very place we need. Give me a few moments and I will provide guidance:
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731616219
I think we will only find out if we have problems after running (and possibly fixing) all the tests (and then possibly running some system tests with real AWS. This is the "individual" part of each such upgrade.
Regarding constraints - if we have a "green" (all tests passing) build in master with such a setup.py change, then constraints will be updated automatically. The condition is that the PR passes all the tests after merging to master -> this automatically triggers constraints update for master. Same with v1-10-stable. Once a test passes in "v1-10-stable" (those are run with "eager" pip update that update to latest version of dependencies matching setup.py) the constraints-1-10 are also updated automatically.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731260231
> @potiuk do I have to make a change in airflow sources to trigger this step or is there other way?
No - the step is queued. Whenever any of the "scripts" changes , all tests are run.
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732371416
@potiuk it seems that we have exactly the same problem but in different place:
https://github.com/turbaszek/airflow/runs/1443863231
~I would say that the problem can be with the image tag `apache/airflow:master-python3.6` against which we run the check. The image that was build and pushed to registry is `docker push docker.pkg.github.com/turbaszek/airflow/master-python3.6:379534179`~
Nope, that image still rises error.
Digging deeper the problem seems to be in the fact that we are using master constraints to build the image (`--build-arg AIRFLOW_CONSTRAINTS_REFERENCE=constraints-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
[GitHub] [airflow] potiuk commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731710155
> So what should I do now? The setup.py was updated, the dependencies in image were not so the check is failing. And because it is failing we won't run tests
Push the fork as master in your own fork. When the test run with your own master it will run eager upgrade check after first installing it, and it will run all tests there
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731261964
> > @potiuk do I have to make a change in airflow sources to trigger this step or is there other way?
>
> No - the step is queued. Whenever any of the "scripts" changes , all tests are run.
Ah. But in this case, the "master" script is run (because building image is done in master). You need to push it to your own fork as master to get it run if you want to check it. 'git push <yourfork> yourbranch:master' (just be careful not to push to Airflow)
But after we merge it, it will run for all PRs.
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731757501
> Push the fork as master in your own fork. When the test run with your own master it will run eager upgrade check after first installing it, and it will run all tests there
I'm doing this since you suggested it in one of the first comments. For example here:
https://github.com/turbaszek/airflow/runs/1438401698?check_suite_focus=true
But the dependencies are still not updated even if the flag is set to true:
https://github.com/turbaszek/airflow/runs/1438439661?check_suite_focus=true
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731802487
I will take a look tomorrow. But this PR aimed to validate only prod image. And I think having this check in the same place where we run other validations for image make sense. And it does not require additional matrix = more jobs to run in parallel.
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732371416
@potiuk it seems that we have exactly the same problem but in different place:
https://github.com/turbaszek/airflow/runs/1443863231
~I would say that the problem can be with the image tag `apache/airflow:master-python3.6` against which we run the check. The image that was build and pushed to registry is `docker push docker.pkg.github.com/turbaszek/airflow/master-python3.6:379534179`~
Nope, that image still rises error.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py/setup.cfg gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI). The github.sha is needed because we want to upgrade as soon as possible
# And not when setup.py changes only. This is because we need to invalidate the layer in docker when we first install the
# pip dependencies for caching (so that we can run the subsequent `pip install http://github'. If we use "SHA" as the value of
# upgradeToLatestConstraints, it will still upgrade them every time wen new commit is merged - otherwise Docker cache is
# used and no upgrade is done.
if push or schedule -> upgradeToLatestConstraints = github.sha
```
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731763991
Sorry for that - but this is extremely hard to debug as there is no indication whatsoever and it takes so long to iterate with GA.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731261964
> > @potiuk do I have to make a change in airflow sources to trigger this step or is there other way?
>
> No - the step is queued. Whenever any of the "scripts" changes , all tests are run.
Ah. But in this case, the "master" script is run (because building image is done in master). You need to push it to your own fork as master to get it run if you want to check it. 'git push <yourfork> yourbranch:master' (just be careful not to Bush to Airflow)
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731775166
> Image build variables:
> UPGRADE_TO_LATEST_CONSTRAINTS: true
> CHECK_IMAGE_FOR_REBUILD: true
> ```
That's good!
IThe problem is that we are running the tests in CI environment and UPGRADE_TO_LATEST_CONSTRAINTS works only in the CI image. The pip-check should be running there!
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732371416
@potiuk it seems that we have exactly the same problem but in different place:
https://github.com/turbaszek/airflow/runs/1443863231
I would say that the problem can be with the image tag `apache/airflow:master-python3.6` against which we run the check. The image that was build and pushed to registry is `docker push docker.pkg.github.com/turbaszek/airflow/master-python3.6:379534179`
----------------------------------------------------------------
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 closed pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #12511:
URL: https://github.com/apache/airflow/pull/12511
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731777833
Proposal:
In ci.yaml we have this: (added some comments):
```
constraints-push:
......
# here we have all the constrains generated in CI image in "artifacts" folder . So in case "UPGRADE_TO_LATEST_CONSTRAINTS" was derived, we should get the new constraints here
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
# you can see how to retrieve the artifacts in the ./scripts/ci/constraints/ci_commit_constraints.sh below
# In order to build image with those new constraints you need to copy the constraints to "./docker-context-files" and
# set AIRFLOW_CONSTRAINTS_LOCATION="./docker-context-files/constraints-${PYTHON_MAJOR_MINOR_VERSION}"
# This will build the production image with those new constraints and then you will be ablet to run `pip check`
"Commit changed constraint files for ${{needs.build-info.outputs.pythonVersions}}"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
```
I think you will have to add a new matrix job (inject it before 'constraint push" and add the same prerequisites). Because we want to run `pip check` for all python versions. Current "constraint_push"
It should have likely few steps similar to those:
```
env:
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
- name: "Extract constraint files to ./docker-context-files
run: .... # Here you need to copy the constraint files from "artifacts" . You want to copy only constraint files as "artifacts" will be far too big (and it takes a lot of time to upload big context to docker envgine before the build)
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
env:
- AIRFLOW_CONSTRAINTS_LOCATION: "./docker-context-files/constraints-...."
```
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731567839
> Upgrading boto3 should update the version of botocore we use
Yup, using `boto3>=1.15.0` should use `botocore>=1.18.0` which does not require docutils. I updated this in setup.py however I'm not sure if this will solve the issues as we probably need update constraints and aws provider @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731764356
BTW. you can see the history of this "support ticket" here: https://github.com/apache/airflow/issues/11294
The answer from GitHub Support on that was:
```
Hopefully we'll have improvements to share regarding this soon.
In the meantime, I'll go ahead and tentatively mark this ticket as solved, however please don't hesitate to reply if I can help with further questions about this request.
```
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731567839
> Upgrading boto3 should update the version of botocore we use
Yup, using `boto3>=1.15.0` should use `botocore>=1.18.0` which does not require docutils. I updated this in setup.py however I'm not sure if this will solve the issues as we probably need update constraints and aws provider?
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733188066
I am fixing some of those in a moment. It is rather easy to do manually.
What I tried to do is to run those in Breeze:
```
pip install --upgrade snowflake
pip install --upgrade moto
pip install --upgrade astroid
```
And after that:
```
pip check
No broken requirements found.
```
seems tht simply downgrading request to 2.23.0 and docutils to 0.15.2 should fix the problem for requests (but there will still be problems for azure that needs fixing.
The problem comes from snowflake's adding the limitation after we upgraded to 2.24.0 it seems. But now, with the latest version of snowflake request will not be upgraded. then we can move on to the others.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731258206
The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run 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] github-actions[bot] commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733881025
[The Workflow run](https://github.com/apache/airflow/actions/runs/383686080) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732133599
[The Workflow run](https://github.com/apache/airflow/actions/runs/378917025) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731261964
> > @potiuk do I have to make a change in airflow sources to trigger this step or is there other way?
>
> No - the step is queued. Whenever any of the "scripts" changes , all tests are run.
Ah. But in this case, the "master" script is run (because building image is done in master). You need to push it to your own fork as master to get it run if you want to check it. 'git push <yourfork> yourbranch:master' (just be careful not to push to Airflow)
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731275495
I got this:
```
botocore 1.17.44 has requirement docutils<0.16,>=0.10, but you have docutils 0.16.
```
and I'm not sure how this should be addressed as the docutils are only devel dependency in setup.py. Should this constraint be added to constraints or to aws section?
https://github.com/PolideaInternal/airflow/runs/1432065455?check_suite_focus=true
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732371416
@potiuk it seems that we have exactly the same problem but in different place:
https://github.com/turbaszek/airflow/runs/1443863231
~I would say that the problem can be with the image tag `apache/airflow:master-python3.6` against which we run the check. The image that was build and pushed to registry is `docker push docker.pkg.github.com/turbaszek/airflow/master-python3.6:379534179`~
Nope, that image still rises error.
Digging deeper the problem seems to be in the fact that we are using master constraints to build the image (`--build-arg AIRFLOW_CONSTRAINTS_REFERENCE=constraints-master`)? And that's something I don't get.
I changed setup.py but the image is still building using old dependencies. Can you explain (or point to docs) how changes to setup.py|cfg gets propagated to prod 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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731802487
I will take a look tomorrow. But this PR aimed to validate only prod image. And I think having this check in the same place where we run other validations for image make sense. And it does not require additional matrix = less jobs to run in parallel.
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732753093
> Digging deeper the problem seems to be in the fact that we are using master constraints to build the image (--build-arg AIRFLOW_CONSTRAINTS_REFERENCE=constraints-master)? And that's something I don't get.
> I changed setup.py but the image is still building using old dependencies. Can you explain (or point to docs) how changes to setup.py|cfg gets propagated to prod images?
Sure. It is something new we want to do, so It needs likely local running first, to see all the details and then transferring that into CI setup with all the right environment variables etc. It has a lot of variable parts, because we have complex constraint requirements and local optimizations that make the image builds faster in the CI - this is why it needs some special treatment,
The technical details and architecture of how images are built are here: https://github.com/apache/airflow/blob/master/IMAGES.rst#technical-details-of-airflow-images. Building production image from the user perspective is described here: https://github.com/apache/airflow/blob/master/docs/production-deployment.rst#production-container-images because this is not a developer, but user-facing feature.
But I perfectly understand it can be overwhelming initially. It's also good to take a look at the Dockerfile: https://github.com/apache/airflow/blob/master/Dockerfile
Basically in the CI environment you will see the `docker build` command that is used to build the image and you can just run it locally and modify to get what you want (with a new thing like this you might also end-up with modifying the Dockrfile itself). Eventually, it ends up with the right combination of the build args and sometimes new features to add.
The installation you mentioned is this: `AIRFLOW_PRE_CACHED_PIP_PACKAGES` , When it is set to true, we are first installing airflow from the current "master". This is an optimization step for rebuilding the image. it will install the "latest master" version of the dependencies so that in subsequent steps it will only incrementally add new dependencies or upgrade them.
```
# In case of Production build image segment we want to pre-install master version of airflow
# dependencies from GitHub so that we do not have to always reinstall it from the scratch.
RUN if [[ ${AIRFLOW_PRE_CACHED_PIP_PACKAGES} == "true" ]]; then \
if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
pip install --user \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
--constraint "https://raw.githubusercontent.com/apache/airflow/${AIRFLOW_CONSTRAINTS_REFERENCE}/constraints-${PYTHON_MAJOR_MINOR_VERSION}.txt" \
&& pip uninstall --yes apache-airflow; \
fi
```
This step can eb entirely skipped by setting `AIRFLOW_PRE_CACHED_PIP_PACKAGES` to "false". In which case the only installation step that will happen is this. Default value of `INSTALL_AIRFLOW_VIA_PIP` is "true" so the next step should be installation of Airflow from this step. And this step should install airflow from the constraints specified.
```
# remove mysql from extras if client is not installed
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_AIRFLOW_VIA_PIP} == "true" ]]; then \
pip install --user "${AIRFLOW_INSTALL_SOURCES}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
if [[ -n "${ADDITIONAL_PYTHON_DEPS}" ]]; then \
pip install --user ${ADDITIONAL_PYTHON_DEPS} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
if [[ ${AIRFLOW_LOCAL_PIP_WHEELS} == "true" ]]; then \
if ls /docker-context-files/*.whl 1> /dev/null 2>&1; then \
pip install --user --no-deps /docker-context-files/*.whl; \
fi ; \
fi; \
find /root/.local/ -name '*.pyc' -print0 | xargs -0 rm -r || true ; \
find /root/.local/ -type d -name '__pycache__' -print0 | xargs -0 rm -r || true
````
So to cut long story short - you need to set AIRFLOW_PRE_CACHED_PIP_PACKAGES to false and AIRFLOW_CONSTRAINTS_LOCATION to your files located in "./docker-context-files"
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI). The last case with github.sha is needed because we want to upgrade as soon as possible
# And not when setup.py changes only. This is because we need to invalidate the layer in docker when we first install the
# pip dependencies for caching (so that we can run the subsequent `pip install http://github'. If we use "SHA" as the value of
# upgradeToLatestConstraints, it will still upgrade them every time wen new commit is merged - otherwise Docker cache is
# used and no upgrade is done.
if push or schedule -> upgradeToLatestConstraints = github.sha
```
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731771740
In this build:
https://github.com/turbaszek/airflow/runs/1438634661
I see:
```
Image build variables:
UPGRADE_TO_LATEST_CONSTRAINTS: true
CHECK_IMAGE_FOR_REBUILD: true
```
but the problem still occurs. Update of constraints is in ci build not in build image workflow, so I'm wodnering if I'm doing changes in right place
----------------------------------------------------------------
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 pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731389311
Upgrading boto3 should update the version of botocore we use
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731777833
Proposal:
In ci.yaml we have this: (added some comments):
```
constraints-push:
......
# here we have all the constrains generated in CI image in "artifacts" folder . So in case if
# "UPGRADE_TO_LATEST_CONSTRAINTS" was derived, we should get the new constraints here
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
# you can see how to retrieve the artifacts in the ./scripts/ci/constraints/ci_commit_constraints.sh below
# In order to build image with those new constraints you need to copy the constraints to "./docker-context-files" and
# set AIRFLOW_CONSTRAINTS_LOCATION="./docker-context-files/constraints-${PYTHON_MAJOR_MINOR_VERSION}"
# This will build the production image with those new constraints and then you will be ablet to run `pip check`
"Commit changed constraint files for ${{needs.build-info.outputs.pythonVersions}}"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
```
I think you will have to add a new matrix job (inject it before 'constraint push" and add the same prerequisites). Because we want to run `pip check` for all python versions. Current "constraint_push"
It should have likely few steps similar to those:
```
env:
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
- name: "Extract constraint files to ./docker-context-files
run: .... # Here you need to copy the constraint files from "artifacts" . You want to copy only constraint files as "artifacts" will be far too big (and it takes a lot of time to upload big context to docker envgine before the build)
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
env:
- AIRFLOW_CONSTRAINTS_LOCATION: "./docker-context-files/constraints-...."
```
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732101430
Thanks @potiuk for explanation. I'm testing suggested approach:
https://github.com/turbaszek/airflow/runs/1441614288
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731258408
@potiuk do I have to make a change in airflow sources to trigger this step or is there other 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731275495
I got this:
```
botocore 1.17.44 has requirement docutils<0.16,>=0.10, but you have docutils 0.16.
```
and I'm not sure how this should be addressed as the docutils are only devel dependency in setup.py. Shut this constraint be added to constraints or to aws section?
https://github.com/PolideaInternal/airflow/runs/1432065455?check_suite_focus=true
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI).
if push or schedule -> upgradeToLatestConstratints = "true"
```
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731777833
Proposal:
In ci.yaml we have this: (added some comments):
```
constraints-push:
......
# here we have all the constrains generated in CI image in "artifacts" folder . So in case if
# "UPGRADE_TO_LATEST_CONSTRAINTS" was derived, we should get the new constraints here
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
# you can see how to retrieve the artifacts in the ./scripts/ci/constraints/ci_commit_constraints.sh below
# In order to build image with those new constraints you need to copy the constraints to "./docker-context-files" and
# set AIRFLOW_CONSTRAINTS_LOCATION="./docker-context-files/constraints-${PYTHON_MAJOR_MINOR_VERSION}"
# This will build the production image with those new constraints and then you will be ablet to run `pip check`
"Commit changed constraint files for ${{needs.build-info.outputs.pythonVersions}}"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
```
I think you will have to add a new matrix job (inject it before 'constraint push" and add the same prerequisites). Because we want to run `pip check` for all python versions. Current "constraint_push"
It should have likely few steps similar to those:
```
env:
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
- name: "Extract constraint files to ./docker-context-files
run: .... # Here you need to copy the constraint files from "artifacts" . You want to copy only constraint files as "artifacts" will be far too big (and it takes a lot of time to upload big context to docker engine before the build)
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
env:
- AIRFLOW_CONSTRAINTS_LOCATION: "./docker-context-files/constraints-...."
- name: "Run PIP check"
run: ...
```
Then you should make the "constraints-push" depend on the above job (and only this job). This way we will only update the constraints in "constraints-master" branch when both CI tests passes, and `pip-check` passes.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py/setup.cfg gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI). The github.sha is needed because we want to upgrade as soon as possible
# And not when setup.py changes only. This is because we need to invalidate the layer in docker when we first install the
# pip dependencies for caching (so that we can run the subsequent `pip install http://github'. If we use "SHA" as the value of
# upgradeToLatestConstraints, it will still upgrade them every time wen new commit is merged - otherwise Docker cache is
# used and no upgrade is done.
if push or schedule -> upgradeToLatestConstraints = github.sha
```
But note again -> this logic in "build-workflow-image" will only work in master so in order to check it and test all the changes, you have to push it to your fork's 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
[GitHub] [airflow] potiuk commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731763675
There is a bug in the code - result of one of the latest refactors (and main reason why I opened a support ticket to GitHub). IT's extremely difficult to see such bugs in GA when you have a typo in name of an output because this is silently ignored:
The "Set upgrade to latest constraints" is not executed because the output has earlier version of the job name,
`steps.cancel.outputs.sourceEvent ` should be `steps.source-run-info.outputs.sourceEvent` similarly as in the previous step!
In this code:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731711038
And we can also think about handling this case automatically. Selective checks should allow that:
If setup.py or setup.cfg change would need to be done: if the setup.py gets modified the "upgradeToLatestConstraints" should be set to "true".
if you look in `build-images-workflow-run,yml` this is the condition now:
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
Maybe that's a good time you implement it :)? We'd have to move it to "selective_checks" because there we set conditional variables based on which files changed. In this case, the logic should be:
```
# Needed by default to prevent regular PRs from failing when transitive dependencies cause problem
upgradeToLatestConstraints = false
# handle the case that a change in setup.py/setup.cfg causes necessity of upgrade
if setup.py or setup.py changed -> upgradeToLatestConstraints = "true"
# we want to keep track of failing transitive dependencies as soo as they appear, therefore we want to attempt to upgrade to
# latest constraints. Also if all test are OK in the "master" build, constraints are then automatically updated to the latest ones
# (but this is a separate step in CI). The last case with github.sha is needed because we want to upgrade as soon as possible
# And not when setup.py changes only. This is because we need to invalidate the layer in docker when we first install the
# pip dependencies for caching (so that we can run the subsequent `pip install http://github'. If we use "SHA" as the value of
# upgradeToLatestConstraints, it will still upgrade them every time wen new commit is merged.
if push or schedule -> upgradeToLatestConstraints = github.sha
```
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731777833
Proposal:
In ci.yaml we have this: (added some comments):
```
constraints-push:
......
# here we have all the constrains generated in CI image in "artifacts" folder . So in case "UPGRADE_TO_LATEST_CONSTRAINTS" was derived, we should get the new constraints here
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
# you can see how to retrieve the artifacts in the ./scripts/ci/constraints/ci_commit_constraints.sh below
# In order to build image with those new constraints you need to copy the constraints to "./docker-context-files" and set AIRFLOW_CONSTRAINTS_LOCATION="./docker-context-files/constraints-${PYTHON_MAJOR_MINOR_VERSION}" . This will build the production image with those new constraints and then you will be ablet to run `pip check`
"Commit changed constraint files for ${{needs.build-info.outputs.pythonVersions}}"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
```
I think you will have to add a new matrix job (inject it before 'constraint push" and add the same prerequisites). Because we want to run `pip check` for all python versions. Current "constraint_push"
It should have likely few steps similar to those:
```
env:
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Get all artifacts (constraints)"
uses: actions/download-artifact@v2
with:
path: 'artifacts'
- name: "Extract constraint files to ./docker-context-files
run: .... # Here you need to copy the constraint files from "artifacts" . You want to copy only constraint files as "artifacts" will be far too big (and it takes a lot of time to upload big context to docker envgine before the build)
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
env:
- AIRFLOW_CONSTRAINTS_LOCATION: "./docker-context-files/constraints-...."
```
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733158598
After discussion with @potiuk we decided to move the check to ci image. And that's are the problem we have:
```
➜ docker run --rm --entrypoint /bin/bash apache/airflow:master-python3.6-ci -c "pip check"
snowflake-connector-python 2.2.10 has requirement azure-storage-blob<13.0.0,>=12.0.0; python_version >= "3.5.2", but you have azure-storage-blob 2.1.0.
snowflake-connector-python 2.2.10 has requirement cryptography<3.0.0,>=2.5.0, but you have cryptography 3.0.
snowflake-connector-python 2.2.10 has requirement idna<2.10, but you have idna 2.10.
snowflake-connector-python 2.2.10 has requirement requests<2.24.0, but you have requests 2.24.0.
moto 1.3.14 has requirement idna<2.9,>=2.5, but you have idna 2.10.
cfn-lint 0.35.0 has requirement importlib-resources~=1.4; python_version < "3.7" and python_version != "3.4", but you have importlib-resources 3.0.0.
botocore 1.17.44 has requirement docutils<0.16,>=0.10, but you have docutils 0.16.
astroid 2.4.2 has requirement lazy-object-proxy==1.4.*, but you have lazy-object-proxy 1.5.1.
```
The main problem is snowflake due to https://github.com/snowflakedb/snowflake-connector-python/issues/324 and this comment:
https://github.com/apache/airflow/blob/ce919912b7ead388c0a99f4254e551ae3385ff50/setup.py#L385-L392
what's more this library has constraint for boto3, `'boto3>=1.4.4,<1.16'`
https://github.com/snowflakedb/snowflake-connector-python/blob/29ad620f7d8bb14f2bb0bc1d95b4dc35b65447bb/setup.py#L195
I'm wondering how we should solve it. Or what should be our criteria when selecting 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733881325
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731834869
> I will take a look tomorrow. But this PR aimed to validate only prod image. And I think having this check in the same place where we run other validations for image makes sense. And it does not require an additional matrix = less jobs to run in parallel.
I am afraid it has to be done in the matrix and using the upgraded constraints from CI builds.
We have quite a different set of dependencies for different python versions. And if a pip check succeeds on one there is no guarantee it will succeed with the other. We also cannot upgrade the constraints when we are building the image for regular PRs - because of our transitive dependency problems.
If we do this, then we go back to the situation we had where PRs start failing because someone released a new version of a transitive library we are using (remember werkzeug drama ?).
The constraint mechanism is specially designed to prevent this case. Do you remember the last time it happened recently for us ? Probably not because we are preventing this from happening and the constraint mechanism does it.
So we have to only run upgrade constraints in case of the "master push/schedule builds" and the constraints are now only really updated after all tests pass for them. And this is the perfect time to do the pip check - not sooner, not later. Because this is the moment where the constraints get updated. And we must do a pip check there, otherwise pip check will start failing in master after such constraint push. So we really have to do the pip-check just before we push updated constraints.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731616523
So the process is simple:
1) we update limitations in setup.py
2) we get PR with those
3) we make the PR green
4) we merge it to master/v1-10-stable and after it is green there constraints are updated
----------------------------------------------------------------
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] removed a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] removed a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732131831
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733208572
It's not that easy though. I found that on 3.7 there is more to it and we might break some tests when we do it. What I am going to do is to add a special case that we will be able to test new set of constraints separately so that we do not break people's code. Bear with me.
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732371416
@potiuk it seems that we have exactly the same problem but in different place:
https://github.com/turbaszek/airflow/runs/1443863231
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731329668
Those cases should be analysed one-by-one and decision should be taken individually in each case I am afraid.
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732753093
> Digging deeper the problem seems to be in the fact that we are using master constraints to build the image (--build-arg AIRFLOW_CONSTRAINTS_REFERENCE=constraints-master)? And that's something I don't get.
> I changed setup.py but the image is still building using old dependencies. Can you explain (or point to docs) how changes to setup.py|cfg gets propagated to prod images?
Sure. It is something new we want to do, so It needs likely local running first, to see all the details and then transferring that into CI setup with all the right environment variables etc. It has a lot of variable parts, because we have complex constraint requirements and local optimizations that make the image builds faster in the CI - this is why it needs some special treatment,
The technical details and architecture of how images are built are here: https://github.com/apache/airflow/blob/master/IMAGES.rst#technical-details-of-airflow-images. Building production image from the user perspective is described here: https://github.com/apache/airflow/blob/master/docs/production-deployment.rst#production-container-images because this is not a developer, but user-facing feature.
But I perfectly understand it can be overwhelming initially. It's also good to take a look at the Dockerfile: https://github.com/apache/airflow/blob/master/Dockerfile
Basically in the CI environment you will see the `docker build` command that is used to build the image and you can just run it locally and modify to get what you want (with a new thing like this you might also end-up with modifying the Dockrfile itself). Eventually, it ends up with the right combination of the build args and sometimes new features to add.
The installation you mentioned is controlled by this: `AIRFLOW_PRE_CACHED_PIP_PACKAGES` , When it is set to true, we are first installing airflow from the current "master". This is an optimization step for rebuilding the image. it will install the "latest master" version of the dependencies so that in subsequent steps it will only incrementally add new dependencies or upgrade them.
```
# In case of Production build image segment we want to pre-install master version of airflow
# dependencies from GitHub so that we do not have to always reinstall it from the scratch.
RUN if [[ ${AIRFLOW_PRE_CACHED_PIP_PACKAGES} == "true" ]]; then \
if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
pip install --user \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
--constraint "https://raw.githubusercontent.com/apache/airflow/${AIRFLOW_CONSTRAINTS_REFERENCE}/constraints-${PYTHON_MAJOR_MINOR_VERSION}.txt" \
&& pip uninstall --yes apache-airflow; \
fi
```
This step can eb entirely skipped by setting `AIRFLOW_PRE_CACHED_PIP_PACKAGES` to "false". In which case the only installation step that will happen is this. Default value of `INSTALL_AIRFLOW_VIA_PIP` is "true" so the next step should be installation of Airflow from this step. And this step should install airflow from the constraints specified.
```
# remove mysql from extras if client is not installed
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_AIRFLOW_VIA_PIP} == "true" ]]; then \
pip install --user "${AIRFLOW_INSTALL_SOURCES}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
if [[ -n "${ADDITIONAL_PYTHON_DEPS}" ]]; then \
pip install --user ${ADDITIONAL_PYTHON_DEPS} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
if [[ ${AIRFLOW_LOCAL_PIP_WHEELS} == "true" ]]; then \
if ls /docker-context-files/*.whl 1> /dev/null 2>&1; then \
pip install --user --no-deps /docker-context-files/*.whl; \
fi ; \
fi; \
find /root/.local/ -name '*.pyc' -print0 | xargs -0 rm -r || true ; \
find /root/.local/ -type d -name '__pycache__' -print0 | xargs -0 rm -r || true
````
So to cut long story short - you need to set AIRFLOW_PRE_CACHED_PIP_PACKAGES to false and AIRFLOW_CONSTRAINTS_LOCATION to your files located in "./docker-context-files"
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-732131831
[The Workflow run](https://github.com/apache/airflow/actions/runs/378910343) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733201466
Indeed this seems to be working on 3.8:
```
pip install --upgrade "requests<2.24.0
pip install --upgrade snowflake-connector-python
pip install --upgrade moto
pip install --upgrade astroid
```
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731275495
I got this:
```
botocore 1.17.44 has requirement docutils<0.16,>=0.10, but you have docutils 0.16.
```
and I'm not sure how this should be addressed as the docutils are only devel dependency in setup.py. Shut this constraint be added to constraints or to aws section?
----------------------------------------------------------------
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] turbaszek edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731627938
So what should I do now? The setup.py was updated, the dependencies in image were not so the check is failing. And because it is failing we won't run tests 😄
----------------------------------------------------------------
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] turbaszek commented on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731627938
So what should I do now? The setup.py was updated, the dependencies in image were not so the check is failing 😄
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731616523
So the process is simple:
1) we update limitations in setup.py
2) we get PR with those
3) we make the PR green
4) we merge it to master/v1-10-stable and after it is green there constraints are updated
J
----------------------------------------------------------------
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 edited a comment on pull request #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-731763675
There is a bug in the code - result of one of the latest refactors (and main reason why I opened a support ticket to GitHub). IT's extremely difficult to see such bugs in GA when you have a typo in name of an output because this is silently ignored:
The "Set upgrade to latest constraints" is not executed because the output has earlier version of the job name,
`steps.cancel.outputs.sourceEvent ` should be `steps.source-run-info.outputs.sourceEvent` similarly as in the previous step!
In this code (this bug is in two lines):
```
- name: "Set upgrade to latest constraints"
id: upgrade-constraints
run: |
if [[ ${{ steps.cancel.outputs.sourceEvent == 'push' ||
steps.cancel.outputs.sourceEvent == 'scheduled' }} == 'true' ]]; then
echo "::set-output name=upgradeToLatestConstraints::${{ github.sha }}"
else
echo "::set-output name=upgradeToLatestConstraints::false"
fi
```
----------------------------------------------------------------
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 #12511: Run pip check to verify production images
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12511:
URL: https://github.com/apache/airflow/pull/12511#issuecomment-733190688
I am double-checking that and pushing new images shortly (BTW. once we add pip check to the pipeline and fix all of those this should never happen in the future).
----------------------------------------------------------------
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