mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 39353: Fixed and added tests for docker image name parsing.
Date Thu, 15 Oct 2015 21:19:21 GMT


> 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 ?
> 
> Ben Mahler wrote:
>     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.
> 
> Jojy Varghese wrote:
>     Agreed. What do you think about using a RE parser?

Hm.. worth considering for sure, the ambiguity around whether the first component is a registry
or a repository component might make it tricky..


- 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message