Re: 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/#review141548 --- Closing this review due to inactivity. Please see our [guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md) for reopening reviews. - Joris Van Remoortere On July 6, 2015, 9: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, 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 > > > 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.
> 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.
> 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.
--- 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.
--- 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.
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.
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.
--- 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
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.
--- 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
