potiuk commented on issue #20960:
URL: https://github.com/apache/airflow/issues/20960#issuecomment-1023123165
Looks good. Few comments:
> build_images::forget_last_answer()
We can skip this one. It has been used in pre-commits and shoudl have been
removed in the last `buildx` refactor. I will remove it shortly. It was used to
remember which answer we gave when "build" pre-commit run, so that "mypy" and
"flake" would not ask the same question again. But right now I opted for simply
"warning" the user and continuing with the pre-commit rather than asking the
question, so this is not needed any more.
> initialization::summarize_build_environment()
We should use `BuildParams` for that (and possibly add stuff that is missing
there). This one shoudl likely be different though for CI and PROD build. Right
now the same "summary" is used for PROD/CI. but PROD build accept many more
params and variables, so we could end up with two different summaries - one for
CI and one for PROD image.
> breeze::prepare_command_file()
This one is a bit too "complex" and counter-intuitive and we **might** want
to simplify it. Initially, I've done it because I wanted to have an easy way to
run the final "docker-compose" command which breeze runs in the same way breeze
does by running this simple "dc" script generated. This is nice to debug and
test but a bit anti-pattern (you generate code that you execute later) and
complex to understand. Since we are in python, I would rather now do it
differently. We have the `--verbose` flag that prints the command we execute,
so rather than setting the variables in the "dc" scripts, I'd pass them to
docker-compose command via -e command. I.e instaad of :
What currently generated `dc_ci` script does:
```
PYTHON_MAJOR_MINOR_VERSION=3.6
COMPOSE_FILE=file1;file2....
docker-compose "${@}"
```
I would simply run:
```
docker-compose run -e PYTHON_MAJOR_MINOR_VERSION=3.6 -f <file_1> -f <file2>
.....
```
which is equivalent.
One more thing the `dc_ci` script does - it checks if the docker-compose is
the right version, so we should also do the check in python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
