Skip to content

Add newlines to the JSON stream functions#4276

Merged
crosbymichael merged 1 commit intomoby:masterfrom
tianon:json-stream-newlines
Feb 27, 2014
Merged

Add newlines to the JSON stream functions#4276
crosbymichael merged 1 commit intomoby:masterfrom
tianon:json-stream-newlines

Conversation

@tianon
Copy link
Copy Markdown
Member

@tianon tianon commented Feb 21, 2014

This makes the JSON streams a lot easier to parse in less well-baked JSON parsers, and no less so in better ones.

Line-based JSON streams are very, very common, where simply chunk-based is not very common at all.

If you want a really pretty way to test this, start the compiled daemon listening on a TCP socket and run something like this (and watch it stream all pretty-like, right before your very eyes):

$ echo -e 'FROM debian\nRUN apt-get update\n' | perl -w -MArchive::Tar -e 'my $tar = Archive::Tar->new; { local $/; $tar->add_data("Dockerfile", <>); } print $tar->write' | curl --data-binary @- --header 'Content-Type: application/x-tar' --dump-header - --no-buffer 'http://localhost:4243/build?rm=1&nocache=1'
HTTP/1.1 100 Continue

HTTP/1.1 200 OK
Content-Type: application/json
Date: Fri, 21 Feb 2014 07:02:54 GMT
Transfer-Encoding: chunked

{"stream":"Step 0 : FROM debian\n"}
{"stream":" ---\u003e b5fe16f2ccba\n"}
{"stream":"Step 1 : RUN apt-get update\n"}
{"stream":" ---\u003e Running in 2e05165d76ef\n"}
{"stream":"Get:1 http://security.debian.org wheezy/updates Release.gpg [836 B]\n"}
{"stream":"Get:2 http://security.debian.org wheezy/updates Release [102 kB]\n"}
{"stream":"Get:3 http://http.debian.net wheezy Release.gpg [1672 B]\n"}
{"stream":"Get:4 http://http.debian.net wheezy-updates Release.gpg [836 B]\n"}
{"stream":"Get:5 http://http.debian.net wheezy Release [168 kB]\n"}
{"stream":"Get:6 http://security.debian.org wheezy/updates/main amd64 Packages [161 kB]\n"}
{"stream":"Get:7 http://http.debian.net wheezy-updates Release [124 kB]\n"}
{"stream":"Get:8 http://http.debian.net wheezy/main amd64 Packages [5845 kB]\n"}
{"stream":"Get:9 http://http.debian.net wheezy-updates/main amd64 Packages/DiffIndex [643 B]\n"}
{"stream":"Get:10 http://http.debian.net wheezy-updates/main amd64 2014-02-15-1443.31.pdiff [547 B]\n"}
{"stream":"Get:11 http://http.debian.net wheezy-updates/main amd64 2014-02-15-1443.31.pdiff [547 B]\n"}
{"stream":"Fetched 6405 kB in 4s (1412 kB/s)\nReading package lists..."}
{"stream":"\n"}
{"stream":" ---\u003e 55f3253bb19d\n"}
{"stream":"Successfully built 55f3253bb19d\n"}
{"stream":"Removing intermediate container 2e05165d76ef\n"}

(see also https://gist.github.com/tianon/9129512 for more masochism along similar lines)

This came about from a discussion on IRC: https://botbot.me/freenode/docker/msg/11203433/

I also looked into swapping the Content-Type returned for these streams to something more like "application/x-json-stream", but that looked like it would be a bit more non-trivial (since the StreamFormatter is used in several places).

@tianon
Add newlines to the JSON stream functions
060da57 
This makes the JSON streams a _lot_ easier to parse in less well-baked JSON parsers, and no less so in better ones.

Line-based JSON streams are very, very common, where simply chunk-based is not very common at all.

Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)
@tianon
Copy link
Copy Markdown
Member Author

tianon commented Feb 21, 2014

/cc @vieux

SvenDowideit added a commit to SvenDowideit/docker that referenced this pull request Feb 21, 2014
@SvenDowideit
small update to API docs to go with moby#4276
f83e553 
Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)
@SvenDowideit SvenDowideit mentioned this pull request Feb 21, 2014
Merged
metalivedev added a commit that referenced this pull request Feb 21, 2014
@metalivedev
Merge pull request #4277 from SvenDowideit/tianon-made-me-doit
0c8c87d 
small update to API docs to go with #4276
@vieux
Copy link
Copy Markdown
Contributor

vieux commented Feb 21, 2014

LGTM

@crosbymichael
Copy link
Copy Markdown
Contributor

crosbymichael commented Feb 27, 2014

LGTM

crosbymichael added a commit that referenced this pull request Feb 27, 2014
@crosbymichael
Merge pull request #4276 from tianon/json-stream-newlines
01dc79b 
Add newlines to the JSON stream functions
@crosbymichael crosbymichael merged commit 01dc79b into moby:master Feb 27, 2014
@tianon tianon deleted the json-stream-newlines branch February 27, 2014 02:42
@ibuildthecloud ibuildthecloud mentioned this pull request Mar 19, 2014
Closed
aanand added a commit to aanand/docker-py that referenced this pull request Apr 23, 2014
@aanand
Universally-compatible reading of chunked streams
6f53987 
Docker introduced newlines in stream output in version 0.9
(moby/moby#4276), but not to all
endpoints - POST /images/create, for example, does not include them.

This reverts to the old, less pleasant implementation of
_stream_helper(), with a manual check for newlines to fix the problem
described in docker#176 and fixed in docker#184, without the accompanying
regression. It should work against Docker 0.8, 0.9 and 0.10, both when
building and when pulling.
@aanand aanand mentioned this pull request Apr 23, 2014
Merged
aanand added a commit to aanand/docker-py that referenced this pull request Apr 23, 2014
@aanand
Universally-compatible reading of chunked streams
eef73d4 
Docker introduced newlines in stream output in version 0.9
(moby/moby#4276), but not to all
endpoints - POST /images/create, for example, does not include them.

This reverts to the old, less pleasant implementation of
_stream_helper(), with a manual check for newlines to fix the problem
described in docker#176 and fixed in docker#184, without the accompanying
regression. It should work against Docker 0.8, 0.9 and 0.10, both when
building and when pulling.
aanand added a commit to aanand/docker-py that referenced this pull request Apr 28, 2014
@aanand
Universally-compatible reading of chunked streams
073fdaf 
Docker introduced newlines in stream output in version 0.9
(moby/moby#4276), but not to all
endpoints - POST /images/create, for example, does not include them.

This reverts to the old, less pleasant implementation of
_stream_helper(), with a manual check for newlines to fix the problem
described in docker#176 and fixed in docker#184, without the accompanying
regression. It should work against Docker 0.8, 0.9 and 0.10, both when
building and when pulling.
@paultag paultag mentioned this pull request Jul 16, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tianon @vieux @crosbymichael