Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors out instead of trying to workaround buggy docker-compose v2 #16989

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jul 14, 2021

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk
Copy link
Member Author

potiuk commented Jul 14, 2021

This is the error printed now (of course changed to 2 version in the commit):
Screenshot 2021-07-14 10 29 53

@potiuk potiuk force-pushed the remove-docker-compose-2-hack branch from f94f004 to 0c622ad Compare July 14, 2021 08:35
@potiuk potiuk closed this Jul 14, 2021
@potiuk potiuk reopened this Jul 14, 2021
@potiuk
Copy link
Member Author

potiuk commented Jul 14, 2021

Triggering the build after constraints update (kms causes problems).

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917
@potiuk potiuk force-pushed the remove-docker-compose-2-hack branch from 0c622ad to 0efe94f Compare July 14, 2021 11:05
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jul 14, 2021
@potiuk
Copy link
Member Author

potiuk commented Jul 14, 2021

I merge it - there's something wrong with KMS tests still, but I believe I know the reason and fix it separately.

@potiuk potiuk merged commit 97ae0f2 into apache:main Jul 14, 2021
@potiuk potiuk deleted the remove-docker-compose-2-hack branch July 14, 2021 11:33
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 16, 2021
The docker-compose v2 parsing of env file bug detected
in apache#16949 and eventual disabling of docker-compose-v2 in apache#16989
found another workaround after discussing with docker-compose
maintainers in docker-archive/compose-cli#1917

The VAR=${VAR} pattern seems to work for all cases for Airflow
because we never rely on the fact if variable is set or not -
we always take into account value of the variable (that is by
design and strictly followed everywhere). Therefore we are
sure this workaround is going to work for us.

However we should remove that workaround once the bug is fixed in
Docker-Compose V2.
potiuk added a commit that referenced this pull request Jul 18, 2021
…16989)

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917

(cherry picked from commit 97ae0f2)
josh-fell pushed a commit to josh-fell/airflow that referenced this pull request Jul 19, 2021
…pache#16989)

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917
kaxil pushed a commit that referenced this pull request Aug 17, 2021
…16989)

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917

(cherry picked from commit 97ae0f2)
jhtimmins pushed a commit that referenced this pull request Aug 17, 2021
…16989)

Docker-Compose v2 Beta has an error in processing environment
variable file which prevents Breeze from running. Until it is
fixed, we are going to print an error, explain how to disable
it and exit - because the workaround introduces more problems
than it solves (passing environment variables to container
is broken partially)

Also see docker-archive/compose-cli#1917

(cherry picked from commit 97ae0f2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants