You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/05/08 02:50:37 UTC
[GitHub] [airflow] fredthomsen opened a new pull request #15731: extra docker-py to 4.4.x resolve docker op issues
fredthomsen opened a new pull request #15731:
URL: https://github.com/apache/airflow/pull/15731
Due to changes in the docker api in api version 1.41, the docker python
client needs an update to properly handle the filter param imagejson
endpoint. Without this fix, the `DockerOperator` will not pull the image
unless `force_pull` is set to True.
closes: #13905
I am assuming the constraint branches get updated via another mechanism, so I validated by running `pip install docker~=4.4.1` in breeze and running the docker provider tests.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #15731: extra docker-py to 4.4.x resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835197520
It makes me nervous as well. Let's see if the test succeed.
FYI: Whenever you update setup.py in your PR, the image is built following a bit different route (it uses --upgrade --upgrade-strategy eager rather than constraints, so we should get an updated docker library.
I think if all the tests pass and if you teste a simple DAG, that should be fine. Docker Operator and Docker Hook are rather straightforward and simple.
Re version - I think ~=4.4.1 is a bit too 'cautious". With our constraint mechanism we can relax it much more. the constraints are never upgraded if tests are failed (we have an automated mechanism that does it - similar to dependabot PRs but a bit more complex due to the constraint mechanism). I'd say I would even try to remove the limit altogether and repeat the tests for the 5.0.0 version which has been released at the beginning of April.
I am happy to make some tests after that as well.
--
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 #15731: extra docker-py to 4.4.x resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835203611
The failures are unlrelated - there was a failing master (and it is fixed now). Can you please rebase this one on top of the latest master (and maybe remove the limit as well :D )?
--
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 #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835889422
Looks good . Merging :)
--
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 #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835491260
[The Workflow run](https://github.com/apache/airflow/actions/runs/823632952) is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.
--
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 #15731: extra docker-py to 4.4.x resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835197520
It makes me nervous as well ;) (but just a bit). Let's see if the test succeed.
FYI: Whenever you update setup.py in your PR, the image is built following a bit different route (it uses --upgrade --upgrade-strategy eager rather than constraints, so we should get an updated docker library.
I think if all the tests pass and if you teste a simple DAG, that should be fine. Docker Operator and Docker Hook are rather straightforward and simple.
Re version - I think ~=4.4.1 is a bit too 'cautious". With our constraint mechanism we can relax it much more. the constraints are never upgraded if tests are failed (we have an automated mechanism that does it - similar to dependabot PRs but a bit more complex due to the constraint mechanism). I'd say I would even try to remove the limit altogether and repeat the tests for the 5.0.0 version which has been released at the beginning of April.
I am happy to make some tests after that as well.
--
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 #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835382375
> @potiuk done. Everything still looked good on my end.
:crossed_fingers:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #15731:
URL: https://github.com/apache/airflow/pull/15731
--
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] fredthomsen commented on pull request #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
fredthomsen commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835365925
>
>
> The failures are unlrelated - there was a failing master (and it is fixed now). Can you please rebase this one on top of the latest master (and maybe remove the limit as well :D )?
@potiuk done. Everything still looked good on my end.
--
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] boring-cyborg[bot] commented on pull request #15731: extra docker-py update resolve docker op issues
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #15731:
URL: https://github.com/apache/airflow/pull/15731#issuecomment-835889492
Awesome work, congrats on your first merged pull 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