> On Feb. 18, 2015, 12:21 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 784
> > <https://reviews.apache.org/r/31125/diff/1/?file=866409#file866409line784>
> >
> >     Why not just strings::split(header, " ")?

Well, no. That's the point of this whole change. strings:split(header, " ") 
will split at each space so you do not end up with a few fields (~7 in regular 
docker) but with one field per character in the header. See 
https://github.com/3rdparty/stout/blob/master/tests/strings_tests.cpp#L166 for 
an example on how this splits.


> On Feb. 18, 2015, 12:21 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 821
> > <https://reviews.apache.org/r/31125/diff/1/?file=866409#file866409line821>
> >
> >     Why spending all this work to just get the name? From the nameIdx (and 
> > perhaps also a nameEndIdx, can't we) can't' we just get the name directly 
> > out of the line?

No, one can not. The width of the output changes based on the data returned. 
e.g.

```bash
# docker ps -a
CONTAINER ID        IMAGE                                       COMMAND         
    CREATED             STATUS                     PORTS               NAMES
5389bc761775        centos:centos7                              "/bin/bash"     
    7 days ago          Exited (127) 7 days ago                        
elegant_almeida                              
d5b5b39e11d8        centos:centos7                              "/bin/bash"     
    7 days ago          Exited (0) 7 days ago                          
sick_einstein               ```

vs.

```bash
# docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             
STATUS              PORTS               NAMES
```

The existing code in the executor happens to work by chance (because the NAMES 
column right now is the last column in the output). When this changes (and it 
will change as this is not part of any official API), the code will no longer 
work. Also, the code simply "splits" the line therefore it will only ever work 
for the last column.


> On Feb. 18, 2015, 12:21 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 793
> > <https://reviews.apache.org/r/31125/diff/1/?file=866409#file866409line793>
> >
> >     This seems to low of a level, VLOG(2)?

If only there were some guidelines as to what level corresponds to what type of 
output...


- Henning


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


On Feb. 17, 2015, 3:52 p.m., Henning Schmiedehausen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31125/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2015, 3:52 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-2357
>     https://issues.apache.org/jira/browse/MESOS-2357
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Parse the docker ps format correctly (identify the columns by the
> header names), then find the NAMES column for the container names and
> parse container names out.
> 
> This ensures that, when docker adds more columns to the ps output,
> the parsing will not break.
> 
> Also issue the --no-trunc command line option to have the output
> columns not truncated by docker.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.cpp 94f16e701f89367b599f0bd8f44c2f1de21a8e20 
> 
> Diff: https://reviews.apache.org/r/31125/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Henning Schmiedehausen
> 
>

Reply via email to