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 63254: Added a helper to test if a resource is a disk of a given type.
Date Mon, 30 Oct 2017 10:38:00 GMT


> On Oct. 30, 2017, 10:21 a.m., Benjamin Bannier wrote:
> > src/common/resources.cpp
> > Lines 1155-1156 (patched)
> > <https://reviews.apache.org/r/63254/diff/2/?file=1872362#file1872362line1155>
> >
> >     I see this in other functions here as well, but would like to understand why
we need to have these assertions here. From just looking at the implementation they do not
seem strictly required.
> >     
> >     If we do want to keep these we should at least clearly document them in the
declaration if we wouldn't want to return e.g., a `Try<bool>` in the first place.
> >     
> >     Similar for the `v1` version.

This is because this is pre reservation refinement format. The master should already validated
and normalized the resources, so this shouldn't be possible. I'd prefer to be just consistent
with others.


- Jie


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


On Oct. 29, 2017, 3:45 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63254/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a helper to test if a resource is a disk of a given type.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3 
>   include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d 
>   src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 
>   src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca 
> 
> 
> Diff: https://reviews.apache.org/r/63254/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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