-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Deprecation: change --stream error to a warning, and update deprecated.md #2809
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2809 +/- ##
==========================================
- Coverage 57.14% 57.13% -0.01%
==========================================
Files 297 297
Lines 18639 18642 +3
==========================================
Hits 10651 10651
- Misses 7129 7132 +3
Partials 859 859 |
cli/command/image/build.go
Outdated
|
|
||
| if options.stream { | ||
| return errors.New("Experimental flag --stream was removed, enable BuildKit instead with DOCKER_BUILDKIT=1") | ||
| _, _ = fmt.Fprint(dockerCli.Err(), `DEPRECATED: The experimental --stream flag is deprecated and the build context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/deprecated/removed/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit in doubt, as the flag is "technically" still there, but yeah from a user's perspective, I guess we could say it's removed.
| if options.stream { | ||
| return errors.New("Experimental flag --stream was removed, enable BuildKit instead with DOCKER_BUILDKIT=1") | ||
| _, _ = fmt.Fprint(dockerCli.Err(), `DEPRECATED: The experimental --stream flag is deprecated and the build context | ||
| will be sent non-streaming. Enable BuildKit instead with DOCKER_BUILDKIT=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Consider using BuildKit instead", maybe with a doc link?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion; I recently added docker/docs#11460, and this would be a good candidate for that. Need to check if we have a good location already to point users to (on how to enable buildkit).
can then add a URL like https://docs.docker.com/go/buildkit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened docker/docs#11621 to implement https://docs.docker.com/go/buildkit/ as a redirect
While performance will be worse, we can safely ignore the --stream
option when used, and print a deprecation warning instead of failing
the build.
With this patch:
echo -e "FROM scratch\nLABEL foo=bar" | docker build --stream -
DEPRECATED: The experimental --stream flag has been removed and the build context
will be sent non-streaming. Enable BuildKit instead with DOCKER_BUILDKIT=1
to stream build context, see https://docs.docker.com/go/buildkit/
Sending build context to Docker daemon 2.048kB
Step 1/2 : FROM scratch
--->
Step 2/2 : LABEL foo=bar
---> Running in 99e4021085b6
Removing intermediate container 99e4021085b6
---> 1a7a41be241f
Successfully built 1a7a41be241f
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker v17.07 introduced an experimental `--stream` flag on `docker build` which allowed the build-context to be incrementally sent to the daemon, instead of unconditionally sending the whole build-context. This functionality has been reimplemented as part of BuildKit, which uses streaming by default and the `--stream` option will be ignored when using the classic builder, printing a deprecation warning instead. Users that want to use this feature are encouraged to enable BuildKit by setting the `DOCKER_BUILDKIT=1` environment variable or through the daemon or CLI configuration files. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
Updated; PTAL |
chris-crone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Relates to #2105
Relates to moby/moby#39983
Addresses docker-archive/docker-ce#660 (comment)
builder: print deprecation warning instead of failing for
--streamWhile performance will be worse, we can safely ignore the
--streamoption when used, and print a deprecation warning instead of failing the build.With this patch:
Deprecation: add experimental docker build --stream option
Docker v17.07 introduced an experimental
--streamflag ondocker buildwhichallowed the build-context to be incrementally sent to the daemon, instead of
unconditionally sending the whole build-context.
This functionality has been reimplemented as part of BuildKit, which uses streaming
by default and the
--streamoption will be ignored when using the classic builder,printing a deprecation warning instead.
Users that want to use this feature are encouraged to enable BuildKit by setting
the
DOCKER_BUILDKIT=1environment variable or through the daemon or CLI configurationfiles.