You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2015/11/23 23:16:52 UTC

Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/
-----------------------------------------------------------

(Updated Nov. 23, 2015, 2:16 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.


Summary (updated)
-----------------

Refactor registry client/puller to avoid JSON and struct.


Bugs: MESOS-3994
    https://issues.apache.org/jira/browse/MESOS-3994


Repository: mesos


Description (updated)
-------

Refactor registry client/puller to avoid JSON and struct.


Diffs
-----

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 3fd84e380f9393a6d46544fb7fc7d24bcb6256ac 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp b17b1b6d9e053af8c80217ea322e99ccab957df1 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7fca99a3b118b0ac06566455ef003daeef7d8157 
  src/tests/containerizer/provisioner_docker_tests.cpp 9ff27083be91985484960a0d48d909b5594937db 

Diff: https://reviews.apache.org/r/39712/diff/


Testing
-------

make check(ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Gilbert Song <so...@gmail.com>.

> On Nov. 23, 2015, 4:30 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp, line 705
> > <https://reviews.apache.org/r/39712/diff/5/?file=1132974#file1132974line705>
> >
> >     We need to do a check so that fs_layers size == history size first right?

Correct. This is checked by validate() inside spec::parse already, so no need to check again.


> On Nov. 23, 2015, 4:30 p.m., Timothy Chen wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 740
> > <https://reviews.apache.org/r/39712/diff/5/?file=1132976#file1132976line740>
> >
> >     What's changed in this JSON? Why do we need to change it?

Three reasons to change it:
1. JSON format is incorrect. There should not be `\"` outside of id/parent pair like \"{ id:... parent:... }\", which also causes a protobuf::parse error.
2. Triple `\\"` is not necessary. Only one backslash is needed.
3. Alignment, more clear.


- Gilbert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/#review107685
-----------------------------------------------------------


On Nov. 23, 2015, 10:52 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39712/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 10:52 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3994
>     https://issues.apache.org/jira/browse/MESOS-3994
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor registry client/puller to avoid JSON and struct.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 22873276a04941c52e4df41750174c86515f1951 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 99a421b21cce8311b729689c4c4099206e85dc1f 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 94b7539b1e1d3b83f0c608d2ac3af9aebbd8b2e6 
>   src/tests/containerizer/provisioner_docker_tests.cpp 88b914d3d4a56a51750a75c0988547e2a3e94f56 
> 
> Diff: https://reviews.apache.org/r/39712/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/#review107685
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp (line 598)
<https://reviews.apache.org/r/39712/#comment166922>

    fix the typo (ono -> on)



src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp (line 600)
<https://reviews.apache.org/r/39712/#comment166924>

    We need to do a check so that fs_layers size == history size first right?



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 195)
<https://reviews.apache.org/r/39712/#comment166928>

    let's call it layerId instead of just id



src/tests/containerizer/provisioner_docker_tests.cpp (line 740)
<https://reviews.apache.org/r/39712/#comment166929>

    What's changed in this JSON? Why do we need to change it?


- Timothy Chen


On Nov. 23, 2015, 10:16 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39712/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3994
>     https://issues.apache.org/jira/browse/MESOS-3994
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor registry client/puller to avoid JSON and struct.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 3fd84e380f9393a6d46544fb7fc7d24bcb6256ac 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp b17b1b6d9e053af8c80217ea322e99ccab957df1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7fca99a3b118b0ac06566455ef003daeef7d8157 
>   src/tests/containerizer/provisioner_docker_tests.cpp 9ff27083be91985484960a0d48d909b5594937db 
> 
> Diff: https://reviews.apache.org/r/39712/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/#review107861
-----------------------------------------------------------

Ship it!


Ship It!

- Timothy Chen


On Nov. 24, 2015, 6:52 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39712/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3994
>     https://issues.apache.org/jira/browse/MESOS-3994
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor registry client/puller to avoid JSON and struct.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 22873276a04941c52e4df41750174c86515f1951 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 99a421b21cce8311b729689c4c4099206e85dc1f 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 94b7539b1e1d3b83f0c608d2ac3af9aebbd8b2e6 
>   src/tests/containerizer/provisioner_docker_tests.cpp 88b914d3d4a56a51750a75c0988547e2a3e94f56 
> 
> Diff: https://reviews.apache.org/r/39712/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/#review107746
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39712]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 24, 2015, 6:52 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39712/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3994
>     https://issues.apache.org/jira/browse/MESOS-3994
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor registry client/puller to avoid JSON and struct.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 22873276a04941c52e4df41750174c86515f1951 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 99a421b21cce8311b729689c4c4099206e85dc1f 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 94b7539b1e1d3b83f0c608d2ac3af9aebbd8b2e6 
>   src/tests/containerizer/provisioner_docker_tests.cpp 88b914d3d4a56a51750a75c0988547e2a3e94f56 
> 
> Diff: https://reviews.apache.org/r/39712/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/
-----------------------------------------------------------

(Updated Nov. 23, 2015, 10:52 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3994
    https://issues.apache.org/jira/browse/MESOS-3994


Repository: mesos


Description
-------

Refactor registry client/puller to avoid JSON and struct.


Diffs (updated)
-----

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 22873276a04941c52e4df41750174c86515f1951 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 99a421b21cce8311b729689c4c4099206e85dc1f 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 94b7539b1e1d3b83f0c608d2ac3af9aebbd8b2e6 
  src/tests/containerizer/provisioner_docker_tests.cpp 88b914d3d4a56a51750a75c0988547e2a3e94f56 

Diff: https://reviews.apache.org/r/39712/diff/


Testing
-------

make check(ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song


Re: Review Request 39712: Refactor registry client/puller to avoid JSON and struct.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39712/#review107674
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39712]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 23, 2015, 10:16 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39712/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3994
>     https://issues.apache.org/jira/browse/MESOS-3994
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor registry client/puller to avoid JSON and struct.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 3fd84e380f9393a6d46544fb7fc7d24bcb6256ac 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp b17b1b6d9e053af8c80217ea322e99ccab957df1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7fca99a3b118b0ac06566455ef003daeef7d8157 
>   src/tests/containerizer/provisioner_docker_tests.cpp 9ff27083be91985484960a0d48d909b5594937db 
> 
> Diff: https://reviews.apache.org/r/39712/diff/
> 
> 
> Testing
> -------
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>