You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/07/01 03:07:25 UTC
Review Request 36071: Add flow diagram for docker containerizer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
Repository: mesos
Description
-------
Add flow diagram for docker containerizer.
Diffs
-----
docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
docs/images/docker_containerizer_flow.jpg PRE-CREATION
Diff: https://reviews.apache.org/r/36071/diff/
Testing
-------
make
Thanks,
Timothy Chen
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90010
-----------------------------------------------------------
Bad patch!
Reviews applied: [36071]
Failed command: ./support/apply-review.sh -n -r 36071
Error:
2015-07-01 05:57:11 URL:https://reviews.apache.org/r/36071/diff/raw/ [1338/1338] -> "36071.patch" [1]
36071.patch:15: new blank line at EOF.
+
error: missing binary patch data for 'docs/images/docker_containerizer_flow.jpg'
error: binary patch does not apply to 'docs/images/docker_containerizer_flow.jpg'
error: docs/images/docker_containerizer_flow.jpg: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On July 1, 2015, 1:07 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 1:07 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Bernd Mathiske <be...@mesosphere.io>.
> On July 7, 2015, 4:51 a.m., Bernd Mathiske wrote:
> > 1. Inconsistent capitalization in box labels.
> > 2. You explain different paths to obtain an "executor pid" and then you checkpoint a "container pid". You lost me there.
> > 3. What happens to the tasks? Is this diagram for the executor only? If so, it is incomplete.
>
> Bernd Mathiske wrote:
> Is this still being worked on?
@tnachen: Is this still being worked on?
- Bernd
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90685
-----------------------------------------------------------
On July 6, 2015, 2:37 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 2:37 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> File Attachments
> ----------------
>
> docker_containerizer_flow.png
> https://reviews.apache.org/media/uploaded/files/2015/07/06/d888a674-17d8-4faf-ab03-f1892537a6e5__docker_containerizer_flow.png
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Bernd Mathiske <be...@mesosphere.io>.
> On July 7, 2015, 4:51 a.m., Bernd Mathiske wrote:
> > 1. Inconsistent capitalization in box labels.
> > 2. You explain different paths to obtain an "executor pid" and then you checkpoint a "container pid". You lost me there.
> > 3. What happens to the tasks? Is this diagram for the executor only? If so, it is incomplete.
Is this still being worked on?
- Bernd
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90685
-----------------------------------------------------------
On July 6, 2015, 2:37 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 2:37 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> File Attachments
> ----------------
>
> docker_containerizer_flow.png
> https://reviews.apache.org/media/uploaded/files/2015/07/06/d888a674-17d8-4faf-ab03-f1892537a6e5__docker_containerizer_flow.png
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90685
-----------------------------------------------------------
1. Inconsistent capitalization in box labels.
2. You explain different paths to obtain an "executor pid" and then you checkpoint a "container pid". You lost me there.
3. What happens to the tasks? Is this diagram for the executor only? If so, it is incomplete.
- Bernd Mathiske
On July 6, 2015, 2:37 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 2:37 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> File Attachments
> ----------------
>
> docker_containerizer_flow.png
> https://reviews.apache.org/media/uploaded/files/2015/07/06/d888a674-17d8-4faf-ab03-f1892537a6e5__docker_containerizer_flow.png
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/
-----------------------------------------------------------
(Updated July 6, 2015, 9:37 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
Repository: mesos
Description
-------
Add flow diagram for docker containerizer.
Diffs
-----
docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
docs/images/docker_containerizer_flow.jpg PRE-CREATION
Diff: https://reviews.apache.org/r/36071/diff/
Testing
-------
make
File Attachments (updated)
----------------
docker_containerizer_flow.png
https://reviews.apache.org/media/uploaded/files/2015/07/06/d888a674-17d8-4faf-ab03-f1892537a6e5__docker_containerizer_flow.png
Thanks,
Timothy Chen
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90242
-----------------------------------------------------------
This diagram does seem to be missing some essential flow-diagram-elements. I would expect something more like /docs/images/fetch_flow.jpg here. As an example, we got a branching decision here which is "Is slave contained in Docker Container" which might show as a diamond. Likewise, the docker inspect should be a loop with a case decision (diamond again). The latter will add a level of detail I wish this diagram had as a whole -- more details please.
- Till Toenshoff
On July 1, 2015, 6:24 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 6:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> File Attachments
> ----------------
>
> flow
> https://reviews.apache.org/media/uploaded/files/2015/07/01/49bd5b6c-510a-4c54-aa8e-e9ec3ebb2451__docker_containerizer_flow.jpg
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/
-----------------------------------------------------------
(Updated July 1, 2015, 6:24 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
Repository: mesos
Description
-------
Add flow diagram for docker containerizer.
Diffs
-----
docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
docs/images/docker_containerizer_flow.jpg PRE-CREATION
Diff: https://reviews.apache.org/r/36071/diff/
Testing
-------
make
File Attachments (updated)
----------------
flow
https://reviews.apache.org/media/uploaded/files/2015/07/01/49bd5b6c-510a-4c54-aa8e-e9ec3ebb2451__docker_containerizer_flow.jpg
Thanks,
Timothy Chen
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Timothy Chen <tn...@apache.org>.
> On July 1, 2015, 3:45 p.m., Till Toenshoff wrote:
> > When trying to apply locally, I get:
> > ```
> > $ ./support/apply-review.sh 36071
> > 2015-07-01 17:43:28 URL:https://reviews.apache.org/r/36071/diff/raw/ [1338/1338] -> "36071.patch" [1]
> > 36071.patch:15: new blank line at EOF.
> > +
> > error: missing binary patch data for 'docs/images/docker_containerizer_flow.jpg'
> > error: binary patch does not apply to 'docs/images/docker_containerizer_flow.jpg'
> > error: docs/images/docker_containerizer_flow.jpg: patch does not apply
> > Failed to apply patch
> > ```
>
> Kapil Arya wrote:
> Same here.
I'm not sure how do I add the binary data as git diff doesn't really show it.
I've manually uploaded the jpg. So it's basically the added text + the new jpg. Please help take a look at both!
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90052
-----------------------------------------------------------
On July 1, 2015, 1:07 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 1:07 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On July 1, 2015, 11:45 a.m., Till Toenshoff wrote:
> > When trying to apply locally, I get:
> > ```
> > $ ./support/apply-review.sh 36071
> > 2015-07-01 17:43:28 URL:https://reviews.apache.org/r/36071/diff/raw/ [1338/1338] -> "36071.patch" [1]
> > 36071.patch:15: new blank line at EOF.
> > +
> > error: missing binary patch data for 'docs/images/docker_containerizer_flow.jpg'
> > error: binary patch does not apply to 'docs/images/docker_containerizer_flow.jpg'
> > error: docs/images/docker_containerizer_flow.jpg: patch does not apply
> > Failed to apply patch
> > ```
Same here.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90052
-----------------------------------------------------------
On June 30, 2015, 9:07 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated June 30, 2015, 9:07 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36071: Add flow diagram for docker containerizer.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36071/#review90052
-----------------------------------------------------------
When trying to apply locally, I get:
```
$ ./support/apply-review.sh 36071
2015-07-01 17:43:28 URL:https://reviews.apache.org/r/36071/diff/raw/ [1338/1338] -> "36071.patch" [1]
36071.patch:15: new blank line at EOF.
+
error: missing binary patch data for 'docs/images/docker_containerizer_flow.jpg'
error: binary patch does not apply to 'docs/images/docker_containerizer_flow.jpg'
error: docs/images/docker_containerizer_flow.jpg: patch does not apply
Failed to apply patch
```
- Till Toenshoff
On July 1, 2015, 1:07 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36071/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 1:07 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add flow diagram for docker containerizer.
>
>
> Diffs
> -----
>
> docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798
> docs/images/docker_containerizer_flow.jpg PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36071/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>