> On March 1, 2017, 10:43 a.m., Benjamin Bannier wrote:
> > I wonder if it would make sense to introduce some more knowledge about role
> > hierarchies here to give the user clearer diagnostics; a role `A/B` could
> > be though of as semantically a role `B` with a parent role `A` and as a
> > user it might make sense to think of both `A` and `B` as roles (this
> > picture works as long we are in the hierarchy below `A`, and do not have
> > roles like `C/B` in the picture).
> >
> > With that we wouldn't talk of _path components_ when validating, but
> > instead directly call each element a role. I believe this might possibly
> > also simplify the implementation, e.g., (pseudocode)
> >
> > def validate(role):
> > # Split `role` keeping empty elements
> > for element in strings.split(role, '/'):
> > # validate `element` with existing non-hierarchical role validation
> >
> > With that validation e.g., a role `A/../B` would yield an `Error("Role name
> > '..' is invalid")`, or a role `A//B` an `Error("Empty role name is
> > invalid")`.
Hmmm, I wonder if this is an improvement: reporting `Error("Empty role name is
invalid")` for `A//B` seems like it would be confusing. If we wanted to be more
verbose / detailed in the errors we report, I'd vote for something like:
`Error("Component '" + element + "' in role '" + role + "' is invalid: ...")`.
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57166/#review167254
-----------------------------------------------------------
On March 2, 2017, 6:12 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57166/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 6:12 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Role names can now contain forward slashes. Each component in a role
> name must now be validated separately.
>
>
> Diffs
> -----
>
> src/common/roles.cpp 31774a9b8f99f5efeed35b1c3e3486e05ca00f6a
> src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1
>
>
> Diff: https://reviews.apache.org/r/57166/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Neil Conway
>
>
