mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 42504: Multiple Disk: Modified 'DiskInfo' stripping logic.
Date Wed, 20 Jan 2016 00:43:58 GMT

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



src/common/resources.cpp (lines 918 - 919)
<https://reviews.apache.org/r/42504/#comment176227>

    We should update the comments here.



src/common/resources.cpp (line 926)
<https://reviews.apache.org/r/42504/#comment176232>

    No need for the 'has_disk()' check before it's already checked above. THe following looks
more clean to me.
    
    ```
    if (stripped.disk().has_source()) {
      ...
    } else {
      ...
    }
    ```



src/common/resources.cpp (lines 961 - 966)
<https://reviews.apache.org/r/42504/#comment176233>

    Do you need to update this as well?



src/tests/persistent_volume_tests.cpp (lines 91 - 106)
<https://reviews.apache.org/r/42504/#comment176253>

    Instead of overwriting CreateSlaveFlags() which is a little implicit, can we add a SlaveResources()
method to return the slave's resources to be more explicit.
    
    ```
    Resources getDiskResources(const Megabytes& mb)
    {
      ...
    }
    
    slaveFlags.resources = getDiskResources(...);
    ```



src/tests/persistent_volume_tests.cpp (lines 576 - 580)
<https://reviews.apache.org/r/42504/#comment176262>

    Wondering if we should introduce an overload of 'createPersistentVolume' that takes resource,
id and containerPath. The might be able to simply your subsequent patch on tests.


- Jie Yu


On Jan. 19, 2016, 3:51 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42504/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactored persistent volume tests to use common slave resources.
> 
> 
> Diffs
> -----
> 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/common/resources_utils.cpp d64db54a175299bf7c32ecbeeb1ce9a7afb1c88a 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/persistent_volume_tests.cpp 7acf7ab29d64d891f3288f8042d267dcc82a32e9 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42504/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


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