Skip to content

Conversation

@gabrielvieira37
Copy link

Fix docker compose version to handle depends_on feature called condition.
Versions above or equal 3 does not support this feature, it did not port healthcheck dependencies.
Change version to to 2.4, the latest version below 3 that support that feature.

@boring-cyborg
Copy link

boring-cyborg bot commented Jun 11, 2021

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@potiuk
Copy link
Member

potiuk commented Jun 11, 2021

According to #15509 This feature is not documented in docker docs, but it works.

@potiuk
Copy link
Member

potiuk commented Jun 11, 2021

@gabrielvieira37 - did you actually have any problems with this option or is just something you noticed/detected by some static checks?

@gabrielvieira37
Copy link
Author

I had a problem when i tried to run the docker-compose.yml that the documentation links. When i tried to run it some errors showed up.
The one that i have mentioned here is that the docker-compose at version 3 or above needs an array on depends_on command.
Instead in airflow compose file it has a condition command inside of depends_on to check its health which can only be performed on versions below 3.
The other problem was about the extension fields x- . It also needs to be version 3.7+ or 2.4+ .
So to solve all of those problems i changed the version to 2.4 and it worked fine :)

@potiuk
Copy link
Member

potiuk commented Jun 11, 2021

cc: @mik-laj

@mik-laj
Copy link
Member

mik-laj commented Jun 11, 2021

What version of Docker compose do you have? Do you use docker compose or docker-conpose command?

We check the compliance of this file with the official Compose specification, which the version field marked as deprecated. Have you tried to delete this field?

@gabrielvieira37
Copy link
Author

gabrielvieira37 commented Jun 12, 2021

docker-compose version 1.25.0. I use docker-compose.
The changes between 1.29.2 (current) and 1.25.0 did not handle those errors.
I followed the airflow tutorial to use that docker-compose file. But it was not running at all.
When I read about it a little bit more I found out the issues that I am talking here.
Have you tried to run that docker-compose file there ?

@mik-laj
Copy link
Member

mik-laj commented Jun 13, 2021

Docker-compose v1.29.1 is required now, becuase we use depends_on with condition field set to service_completed_successfully . See: #16180

Have you tried to run that docker-compose file there ?

I'm testing this file on my CI and I didn't notice any problem.
https://github.com/mik-laj/airflow-docker-compose-examples

@potiuk
Copy link
Member

potiuk commented Jun 23, 2021

Hey @gabrielvieira37 - have you tested with 1.29.* ? Do you still see the problem? I am closing the issue now unless you come back and tell us that it did not work with 1.29 (I also tested with 1.29 and it works just fine).

@potiuk potiuk closed this Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants