> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioner/docker/message.hpp, line 46 > > <https://reviews.apache.org/r/39353/diff/1/?file=1099046#file1099046line46> > > > > How about handling parse errors ? Maybe change this to a Try?
Agreed, did you see the TODO..? ``` // TODO(bmahler): Validate based on docker's validation logic // and return a Try here. ``` > On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioner/docker/message.hpp, line 51 > > <https://reviews.apache.org/r/39353/diff/1/?file=1099046#file1099046line51> > > > > Would this also allow @@ or @@@ ? Wondering if we can use a regular > > expression parser for parsing ? Yes, this won't reject invalid input (given this doesn't return a Try). The first step (this change) is to fix our code to accept valid input (which we weren't doing for all cases!). I've left the TODO for validation and rejecting bad input. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39353/#review102816 ----------------------------------------------------------- On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39353/ > ----------------------------------------------------------- > > (Updated Oct. 15, 2015, 7:17 p.m.) > > > Review request for mesos, Jojy Varghese and Timothy Chen. > > > Repository: mesos > > > Description > ------- > > The existing docker image parsing did not work in many cases: if a digest is > present, a registry port is included. And it could not resolve the ambiguity > that exists between the registry and the repository. > > This follows docker's approach to parsing, which is a bit hacky due to the > way the registry and repository are disambiguated, see the comments. > > > Diffs > ----- > > include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 > src/slave/containerizer/provisioner/docker/message.hpp > 6368bf4caec6f8c3ac97282f41c55381f920bce9 > src/slave/containerizer/provisioner/docker/message.proto > bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 > src/slave/containerizer/provisioner/docker/store.cpp > 637c97c0973bda492826803a962c99635647692d > src/tests/containerizer/provisioner_docker_tests.cpp > 9c3c45a81be6398722a37911788e347a4e91cce8 > > Diff: https://reviews.apache.org/r/39353/diff/ > > > Testing > ------- > > Added a test. > > > Thanks, > > Ben Mahler > >
