mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 52001: Allow text based disk resources to indicate source type and root.
Date Tue, 20 Sep 2016 21:34:23 GMT

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


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