potiuk commented on issue #4543: [AIRFLOW-3718] [WIP] Multi-layered version of the docker image URL: https://github.com/apache/airflow/pull/4543#issuecomment-473731432 @Fokko Thanks for comments :) really appreciate! I like that you liked breeze. We find our version of breeze pretty indispensable for our daily work. > Remove the tox from .flake8 Will do. Good catch. > Why are all the versions changed in airflow/www/package-lock.json, maybe revert this? I see also that we're going back to lower versions. What do you think? Already reverted. That was automatically done by 'npm install'. This is the default behaviour of NPM and something that maybe long term we would like to have with setup.py/requirements - exactly what I advocated for some time ago - have a fixed set of deps (lock) and update them automatically during development (npm install) so that it can be committed and used in CI environment ('npm ci'). I find this workflow very stable. But that's a side comment. We have something similar now with the layered build (as the requirements are frozen in CI image in-between setup.py changes). Something that we should discuss if community is fine with. I will re-open the discussion as I am ready to do so. > Personally I would leave the .bash_aliases and the .inputrc out, since these settings are personal preferences. No problem, I will remove them, though I love to have those inside the docker. What I can do instead, I can implement conditional mounting of those two in case I place those files in a specific directory from sources. This way I can keep them "local" for myself but still get them used every time I enter the environment. > For the go docker-show-context stuff, shoudn't we able to use a binary, instead of adding the source to the Airflow repository. I just removed it. I needed that only temporarily while I debugged some missing .dockerignores. I found a better solution now with the docker-show-all-context-files, which is kinda nice - it is controlled with an environment variable and it will save list of all files that are in Docker context in a file in "dist" directory. It's kind a smart and super-simple (found it on Stack Overflow somewhere)- it creates an empty container with the same context and lists all files from the container :) > What do you think of adding hadolint to the pre-steps of the CI? Great idea. I will give it a spin! Looks cool. > Further thoughts, ideally we should have one image for testing Airflow. So in that case we should deprecate https://github.com/apache/airflow-ci and use this image for testing as well. Absolutely. This is what already happens in this change and one of the biggest reasons to implement it. The CI scripts are building the image locally (using cache from DockerHub and local sources to incrementally add latest sources/or packages - whatever has changed) and then use those locally built images to run the tests (not the incubator-ci one)
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
