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



Given these comments, do you want to punt on improving the simple string 
parsing and get the JSON format working first?

Also we should have independent unit tests here in resources_tests.cpp, this 
shouldn't to be coupled with the containerizer.


src/common/resources.cpp (lines 610 - 614)
<https://reviews.apache.org/r/52001/#comment217408>

    What we need here is a tokenize method that scan from the string backwards 
(perhaps add an argument `bool reverse = false` to tokenize) which shouldn't be 
hard. Then we can tokenize it with `maxTokens=2` to get a pair of (key, value), 
then go into the key and value themselves for further parsing. The key and 
value are separately structured, so it makes more sense to "divide and conquer".
    
    Inside the key, we should decide that the strings inside `[]` is 
`DiskInfo::Source` and parse it as such. i.e., perhaps with an internal helper:
    
    ```
    Try<DiskInfo::Source> parseDiskSource(string text);
    ```



src/common/resources.cpp (line 629)
<https://reviews.apache.org/r/52001/#comment217415>

    s/diskOpenParam/openBracket/ ?
    
    Same for closeBracket, this is pretty similar to openParen and closeParen 
in this method.
    
    For the string between the brackets, we know we want to parse it as 
`DiskSource::Info`.



src/common/resources.cpp (lines 678 - 701)
<https://reviews.apache.org/r/52001/#comment217414>

    This could be natrually encapsulated in a
    
    ```
    Try<DiskInfo::Source> parseDiskSource(string text);
    ```


- Jiang Yan Xu


On Sept. 19, 2016, 3:43 p.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52001/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
>     https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Text based representation of disk resources can indicate source type
> (ie. PATH or MOUNT) and its root. This makes it closer to the JSON
> representation.
> 
> 
> Diffs
> -----
> 
>   src/common/resources.cpp 4602bff22ec7ff77f069056085ad92bfa04595f3 
>   src/v1/resources.cpp 7c4db44d4c31a8295cc16b0ce1ef5adf314e6ac6 
> 
> Diff: https://reviews.apache.org/r/52001/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Reply via email to