----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40544/#review107995 -----------------------------------------------------------
1. General fly by style comments. 2. Have we thought about moving the request validations to a central place preferably in `src/master/validation.cpp` ? src/master/master.hpp (line 861) <https://reviews.apache.org/r/40544/#comment167351> We generally avoid redundant/self-explanatory comments. Can we remove this ? src/master/quota_handler.cpp (line 362) <https://reviews.apache.org/r/40544/#comment167354> Can we put the `request.url.path` in Quotes ('') similar to other instances below ? src/master/quota_handler.cpp (line 374) <https://reviews.apache.org/r/40544/#comment167364> When can a valid request path have more then 3 tokens ? I wonder if it would just be fine for this to be exact equality check for 3 parts i.e. `!=3` ? src/master/quota_handler.cpp (line 377) <https://reviews.apache.org/r/40544/#comment167353> Let's not return speculative errors back to the end-user. How about: ": Expected atleast 3 but found only " + tokens.size() + " tokens" src/master/quota_handler.cpp (line 383) <https://reviews.apache.org/r/40544/#comment167357> Remove period at the end. Also how about: ": Missing 'quota' endpoint" ? src/master/quota_handler.cpp (line 386) <https://reviews.apache.org/r/40544/#comment167352> `const string&` Also, what happens if this is an unknown role for the master ? src/master/quota_handler.cpp (line 391) <https://reviews.apache.org/r/40544/#comment167358> 1. Remove period at the end. 2. s/Can not remove quota for a role that has no quota/Role has no quota set src/master/quota_handler.cpp (line 394) <https://reviews.apache.org/r/40544/#comment167359> Kill this redundant comment. - Anand Mazumdar On Nov. 22, 2015, 1:35 p.m., Joerg Schad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40544/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2015, 1:35 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joris Van Remoortere. > > > Bugs: MESOS-3073 > https://issues.apache.org/jira/browse/MESOS-3073 > > > Repository: mesos > > > Description > ------- > > Added quota remove handling. > > > Diffs > ----- > > src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 > src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f > > Diff: https://reviews.apache.org/r/40544/diff/ > > > Testing > ------- > > Test are in the next review. > > > Thanks, > > Joerg Schad > >
