mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rojas <alexan...@mesosphere.io>
Subject Re: Review Request 44322: Implemented a generalized interface for the authorizer.
Date Wed, 09 Mar 2016 09:22:51 GMT


> On March 9, 2016, 10:18 a.m., Adam B wrote:
> > src/master/master.cpp, line 2801
> > <https://reviews.apache.org/r/44322/diff/5/?file=1292969#file1292969line2801>
> >
> >     Someday we may want to return something besides a bool, so that the client can
get back a more meaningful error than "Forbidden", e.g. which of the N roles specified was
disallowed?
> >     Any ideas how we would do that for the operator endpoints?

I once used an `Error<Nothing>` as return value. We can check how boost implements its
`error_code` I found their solution elegant and I use it in all my private projects (not that
I have many anymore).


> On March 9, 2016, 10:18 a.m., Adam B wrote:
> > src/master/master.cpp, line 2837
> > <https://reviews.apache.org/r/44322/diff/5/?file=1292969#file1292969line2837>
> >
> >     When would this ever be empty? Shouldn't we have validated somewhere prior that
the Reserve operation is reserving at least one resource/role? I'd think this could be a CHECK.

Well, this version is consistent with the current semantics. I think having non resource is
equivalent to request for all of them.


> On March 9, 2016, 10:18 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 82
> > <https://reviews.apache.org/r/44322/diff/5/?file=1292961#file1292961line82>
> >
> >     Can we get a comment?

there will be someā€¦ it's in the TODO list in de description.


- Alexander


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


On March 9, 2016, 6:07 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44322/
> -----------------------------------------------------------
> 
> (Updated March 9, 2016, 6:07 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-2950
>     https://issues.apache.org/jira/browse/MESOS-2950
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> **WIP: Do not commit**
> 
> Implements the later [Generic Authorizer Interface v 0.3.1](https://docs.google.com/document/d/1-XARWJFUq0r_TgRHz_472NvLZNjbqE4G8c2JL44OSMQ)
> proposal.
> 
> It still lacks some parts:
> 
> - [ ] Doxygen in the interface.
> - [ ] Updating `authorizer.md`
> 
> Still the basic functionality is there and I don't expect it to change
> much. As I mentioned, comments aren't there yet but feel free to point
> where they are missed, at this point focusing in the actual content
> may not be relevant as the patch may change from its current form.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.hpp ec6c9928c55c3096c7de634f900419abbdd00886 
>   include/mesos/authorizer/authorizer.proto PRE-CREATION 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
>   src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
>   src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
>   src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/quota_handler.cpp a41c91f10bc0eedc754425b4de1b3e184c4ffb08 
>   src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
>   src/tests/master_authorization_tests.cpp 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/mesos.hpp 9c62833e0a64cfd62fce8cffd04f9cdd933646c8 
>   src/tests/mesos.cpp 395b23d32b2d03aef446858e197cb9788644eefa 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
> 
> Diff: https://reviews.apache.org/r/44322/diff/
> 
> 
> Testing
> -------
> 
> make -j4 check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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