Skip to content

Much easier to use and better documented Docker image#14911

Merged
potiuk merged 1 commit into
apache:masterfrom
potiuk:default-constraints-location-for-images
Mar 23, 2021
Merged

Much easier to use and better documented Docker image#14911
potiuk merged 1 commit into
apache:masterfrom
potiuk:default-constraints-location-for-images

Conversation

@potiuk

@potiuk potiuk commented Mar 20, 2021

Copy link
Copy Markdown
Member

Previously you had to specify AIRFLOW_VERSION_REFERENCE and
AIRFLOW_CONSTRAINTS_REFERENCE to point to the right version
of Airflow. Now those values are auto-detected if not specified
(but you can still override them)

This change allowed to simplify and restructure the Dockerfile
documentation - following the recent change in separating out
the docker-stack, production image building documentation has
been improved to reflect those simplifications. It should be
much easier to grasp by the novice users now - very clear
distinction and separation is made between the two types of
building your own images - customizing or extending - and it
is now much easier to follow examples and find out how to
build your own image. The criteria on which approach to
choose were put first and forefront.

Examples have been reviewed, fixed and put in a logical
sequence. From the most basic ones to the most advanced,
with clear indication where the basic aproach ends and where
the "power-user" one starts. The examples were also separated
out to separate files and included from there - also the
example Docker images and build commands are executable
and tested automatically in CI, so they are guaranteed
to work.

Finally The build arguments were split into sections - from most
basic to most advanced and each section links to appropriate
example section, showing how to use those parameters.

Fixes: #14848
Fixes: #14255


^ 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

potiuk commented Mar 20, 2021

Copy link
Copy Markdown
Member Author

@ecerulm @grantav -> your issues will be fixed when this one is merged.

@potiuk potiuk force-pushed the default-constraints-location-for-images branch 3 times, most recently from e92e71f to e550565 Compare March 21, 2021 08:01
@potiuk

potiuk commented Mar 21, 2021

Copy link
Copy Markdown
Member Author

@mik-laj - that impacts a bit #14846 - If you want you can merge it first and I will resolve the conflicts after (but also need review here)

@potiuk potiuk force-pushed the default-constraints-location-for-images branch 2 times, most recently from d4e66cc to aea5a2e Compare March 21, 2021 18:45
@potiuk potiuk changed the title Constraints location is now calculated automatically Much esier to use and documented Docker image Mar 21, 2021
@potiuk potiuk changed the title Much esier to use and documented Docker image Much easier to use and documented Docker image Mar 21, 2021
@potiuk potiuk force-pushed the default-constraints-location-for-images branch from aea5a2e to d961399 Compare March 21, 2021 18:45
@potiuk

potiuk commented Mar 21, 2021

Copy link
Copy Markdown
Member Author

Hey @ecerulm @grantav @mik-laj (also @kaxil and @ashb) - while improving the automation of image building, I also realized that after some of the recent changes we got very close to a "target" prod image setting I Imagined when I started it. Finally the original "development" image turned into a much closer "production" one - much more user friendly to build and use.

The nice thing is that we still kept the "development" friendliness (and especially CI friendliness) of the image that I started it from and we have now a very nice (I think) way of describing it. Following @mik-laj separating out of the docker-stack in our documentation I restructured it internally and removed some of the old arguments/fixed examples, I also made a very clear progression of complexity of building the image - from very easy extending the image going through more complex cases, ending with rather complex "building image in restricted environment" (which was BTW a real case we had with two of our customers independently so it is not an isolated or rare case).

I restructured and split the "build-args" reference following the same pattern - first you have the basic args that you will almost always use when customizing the image, and then you have other args that are grouped by "customization cases" with links to the examples section where you can see how the args can be used.

I made it very clear - I think when extension is a good choice, and when you should go with the customization route. I prepared an example Customize vs Extend where I got 1.1 GB "extended" vs 874 MB "Customized" with the same customization, this is a nice example of showing how you can get 20% optimization if you are a "Power-user".

Moreover - I extracted all the examples (using exampleinclude) and I run them in CI (!) so that we will be sure that those examples work (I have to fix failing test but this is the goal to keep them tested automatically).

@ecerulm -> you were looking at the image recently as the user, I would really appreciate your feedback and potential improvements in this PR - but anyone else's comments are most welcome.

@potiuk potiuk force-pushed the default-constraints-location-for-images branch from d961399 to db89b40 Compare March 21, 2021 19:04
@potiuk potiuk changed the title Much easier to use and documented Docker image Much easier to use and better documented Docker image Mar 21, 2021
@potiuk potiuk force-pushed the default-constraints-location-for-images branch 8 times, most recently from bc38615 to d775d0f Compare March 22, 2021 22:52
Comment thread scripts/ci/images/ci_run_prod_image_test.sh Outdated
Comment thread scripts/docker/common.sh Outdated
Comment thread scripts/docker/common.sh Outdated
@potiuk potiuk force-pushed the default-constraints-location-for-images branch from 0a89d72 to a0840cf Compare March 23, 2021 01:02
@potiuk

potiuk commented Mar 23, 2021

Copy link
Copy Markdown
Member Author

All fixed @mik-laj - I also rebased it on top of #14944 that fixes default group for the airflow user

@github-actions

Copy link
Copy Markdown
Contributor

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 master 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 Mar 23, 2021
Comment thread .github/workflows/ci.yml Outdated
@potiuk potiuk force-pushed the default-constraints-location-for-images branch 3 times, most recently from a322fd8 to 61c5cb7 Compare March 23, 2021 02:21
Previously you had to specify AIRFLOW_VERSION_REFERENCE and
AIRFLOW_CONSTRAINTS_REFERENCE to point to the right version
of Airflow. Now those values are auto-detected if not specified
(but you can still override them)

This change allowed to simplify and restructure the Dockerfile
documentation - following the recent change in separating out
the docker-stack, production image building documentation has
been improved to reflect those simplifications. It should be
much easier to grasp by the novice users now - very clear
distinction and separation is made between the two types of
building your own images - customizing or extending - and it
is now much easier to follow examples and find out how to
build your own image. The criteria on which approach to
choose were put first and forefront.

Examples have been reviewed, fixed and put in a logical
sequence. From the most basic ones to the most advanced,
with clear indication where the basic aproach ends and where
the "power-user" one starts. The examples were also separated
out to separate files and included from there - also the
example Docker images and build commands are executable
and tested automatically in CI, so they are guaranteed
to work.

Finally The build arguments were split into sections - from most
basic to most advanced and each section links to appropriate
example section, showing how to use those parameters.

Fixes: apache#14848
Fixes: apache#14255
@potiuk potiuk force-pushed the default-constraints-location-for-images branch from 61c5cb7 to f74278c Compare March 23, 2021 03:10
@potiuk potiuk merged commit 3eb67af into apache:master Mar 23, 2021
@potiuk potiuk deleted the default-constraints-location-for-images branch March 23, 2021 03:13
potiuk added a commit that referenced this pull request Mar 23, 2021
Previously you had to specify AIRFLOW_VERSION_REFERENCE and
AIRFLOW_CONSTRAINTS_REFERENCE to point to the right version
of Airflow. Now those values are auto-detected if not specified
(but you can still override them)

This change allowed to simplify and restructure the Dockerfile
documentation - following the recent change in separating out
the docker-stack, production image building documentation has
been improved to reflect those simplifications. It should be
much easier to grasp by the novice users now - very clear
distinction and separation is made between the two types of
building your own images - customizing or extending - and it
is now much easier to follow examples and find out how to
build your own image. The criteria on which approach to
choose were put first and forefront.

Examples have been reviewed, fixed and put in a logical
sequence. From the most basic ones to the most advanced,
with clear indication where the basic aproach ends and where
the "power-user" one starts. The examples were also separated
out to separate files and included from there - also the
example Docker images and build commands are executable
and tested automatically in CI, so they are guaranteed
to work.

Finally The build arguments were split into sections - from most
basic to most advanced and each section links to appropriate
example section, showing how to use those parameters.

Fixes: #14848
Fixes: #14255
potiuk added a commit that referenced this pull request Mar 25, 2021
Previously you had to specify AIRFLOW_VERSION_REFERENCE and
AIRFLOW_CONSTRAINTS_REFERENCE to point to the right version
of Airflow. Now those values are auto-detected if not specified
(but you can still override them)

This change allowed to simplify and restructure the Dockerfile
documentation - following the recent change in separating out
the docker-stack, production image building documentation has
been improved to reflect those simplifications. It should be
much easier to grasp by the novice users now - very clear
distinction and separation is made between the two types of
building your own images - customizing or extending - and it
is now much easier to follow examples and find out how to
build your own image. The criteria on which approach to
choose were put first and forefront.

Examples have been reviewed, fixed and put in a logical
sequence. From the most basic ones to the most advanced,
with clear indication where the basic aproach ends and where
the "power-user" one starts. The examples were also separated
out to separate files and included from there - also the
example Docker images and build commands are executable
and tested automatically in CI, so they are guaranteed
to work.

Finally The build arguments were split into sections - from most
basic to most advanced and each section links to appropriate
example section, showing how to use those parameters.

Fixes: #14848
Fixes: #14255
potiuk added a commit to potiuk/airflow that referenced this pull request Mar 25, 2021
The change apache#14911 had a bug - when PYTHON_MINOR_MAJOR_VERSION
was removed from the imge args, the replacement `python -m site`
expression missed `/airflow/` suffix. Unfortunately it was not
flagged as an error because the recompiling script silently
skipped recompilation step in such case.

This change:

* fixes the error
* removes the silent-skipping if check (the recompilation will
  fail in case it is wrongly set)
* adds check at image verification whether dist/manifest.json is
  present.

Fixes: apache#14991
kaxil pushed a commit that referenced this pull request Mar 25, 2021
The change #14911 had a bug - when PYTHON_MINOR_MAJOR_VERSION
was removed from the imge args, the replacement `python -m site`
expression missed `/airflow/` suffix. Unfortunately it was not
flagged as an error because the recompiling script silently
skipped recompilation step in such case.

This change:

* fixes the error
* removes the silent-skipping if check (the recompilation will
  fail in case it is wrongly set)
* adds check at image verification whether dist/manifest.json is
  present.

Fixes: #14991
potiuk added a commit that referenced this pull request Mar 25, 2021
The change #14911 had a bug - when PYTHON_MINOR_MAJOR_VERSION
was removed from the imge args, the replacement `python -m site`
expression missed `/airflow/` suffix. Unfortunately it was not
flagged as an error because the recompiling script silently
skipped recompilation step in such case.

This change:

* fixes the error
* removes the silent-skipping if check (the recompilation will
  fail in case it is wrongly set)
* adds check at image verification whether dist/manifest.json is
  present.

Fixes: #14991
ashb pushed a commit that referenced this pull request Apr 15, 2021
Previously you had to specify AIRFLOW_VERSION_REFERENCE and
AIRFLOW_CONSTRAINTS_REFERENCE to point to the right version
of Airflow. Now those values are auto-detected if not specified
(but you can still override them)

This change allowed to simplify and restructure the Dockerfile
documentation - following the recent change in separating out
the docker-stack, production image building documentation has
been improved to reflect those simplifications. It should be
much easier to grasp by the novice users now - very clear
distinction and separation is made between the two types of
building your own images - customizing or extending - and it
is now much easier to follow examples and find out how to
build your own image. The criteria on which approach to
choose were put first and forefront.

Examples have been reviewed, fixed and put in a logical
sequence. From the most basic ones to the most advanced,
with clear indication where the basic aproach ends and where
the "power-user" one starts. The examples were also separated
out to separate files and included from there - also the
example Docker images and build commands are executable
and tested automatically in CI, so they are guaranteed
to work.

Finally The build arguments were split into sections - from most
basic to most advanced and each section links to appropriate
example section, showing how to use those parameters.

Fixes: #14848
Fixes: #14255
ashb pushed a commit that referenced this pull request Apr 15, 2021
The change #14911 had a bug - when PYTHON_MINOR_MAJOR_VERSION
was removed from the imge args, the replacement `python -m site`
expression missed `/airflow/` suffix. Unfortunately it was not
flagged as an error because the recompiling script silently
skipped recompilation step in such case.

This change:

* fixes the error
* removes the silent-skipping if check (the recompilation will
  fail in case it is wrongly set)
* adds check at image verification whether dist/manifest.json is
  present.

Fixes: #14991
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 kind:documentation

Projects

None yet

3 participants