mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 42476: Introduced protobuf for set quota requests.
Date Wed, 20 Jan 2016 12:11:01 GMT


> On Jan. 20, 2016, 9:14 a.m., Bernd Mathiske wrote:
> > docs/quota.md, line 94
> > <https://reviews.apache.org/r/42476/diff/3/?file=1201682#file1201682line94>
> >
> >     plural
> 
> Alexander Rukletsov wrote:
>     I think we use single everywhere. It means total quota guarantee and shouldn't necessarily
resemble the fact that there are multiple resources objects backing it. Moreover, it will
require us to update `QuotaInfo` and `QuotaRequest` protos as well.
> 
> Bernd Mathiske wrote:
>     I would not have seen it this way if the comment describing the field in QuotaRequest
did not make me believe this was plural. Now I see your point. However, in that case, I would
suggest making this perspectiove clear at the field definition's comment. Maybe:
>     
>         // The requested guarantee that these resources will be allocatable for the above
role.

Good idea, I'll update the comment.

My biggest concern is that "guarantees" sounds misleading to me. There is a single guarantee
per role, which may comprise various resources.

Moreover, though we indeed almost always prefer plural for repeated fields, singular is considered
a good practice as well: https://developers.google.com/protocol-buffers/docs/cpptutorial


- Alexander


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


On Jan. 20, 2016, 11:58 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42476/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4410
>     https://issues.apache.org/jira/browse/MESOS-4410
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/quota.md 1a6d2f07fb74d168a7eb30764ab9ff80cea5e3b6 
>   include/mesos/quota/quota.proto 338412ee967e14aa1957a47f4a50f2e19e4eca79 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/role_tests.cpp 979391306e2427aaa63a5df32704913f79e20e36 
> 
> Diff: https://reviews.apache.org/r/42476/diff/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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