> On June 9, 2015, 2:32 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1921 > > <https://reviews.apache.org/r/33295/diff/10/?file=980632#file980632line1921> > > > > Just 'pid' will do :)
`pid` conflicts with a name before. However, following the parameter name in the `ProcessBase` constructor now is called `id` > On June 9, 2015, 2:32 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1994 > > <https://reviews.apache.org/r/33295/diff/10/?file=980632#file980632line1994> > > > > Ditto, just "pid" is what we've been calling process ids as variables > > throughout the code. `pid` conflicts with a name before. However, following the parameter name in the `ProcessBase` constructor now is called `id` > On June 9, 2015, 2:32 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/include/process/firewall.hpp, lines 54-56 > > <https://reviews.apache.org/r/33295/diff/10/?file=980629#file980629line54> > > > > Hm.. I think this interface might prove to be a bit confusing for folks: > > > > ``` > > Try<Nothing> apply = rule.apply(...); > > > > if (apply.isError()) { > > // What does this mean? > > } > > ``` > > > > What does it mean for there the rule application to fail? > > > > It's subtle, but much like we did for the validation code, it seems > > clearer to have the rule application return an optional error. The idea is > > that rather than trying to apply and failing, applying the rule always > > succeeds, but yields an error (ideally a 'Rejection', but to avoid the > > unnecessary extra struct): > > > > ``` > > Option<Error> apply = rule.apply(...); > > > > if (apply.isSome()) { > > // This rule rejected the request, when applied. > > } > > ``` > > > > Does that make sense? As you agreed with Till, whe go for `Option<Error>` and do `Option<Error> rejected = rule.apply(...)` - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review87100 ----------------------------------------------------------- On June 8, 2015, 12:09 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33295/ > ----------------------------------------------------------- > > (Updated June 8, 2015, 12:09 p.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till > Toenshoff. > > > Bugs: MESOS-2620 > https://issues.apache.org/jira/browse/MESOS-2620 > > > Repository: mesos > > > Description > ------- > > Introduces the interface `FirewallRule` which will be matched against > incoming connections in order to allow them to be served or being blocked. > For details, check the [design > doc](https://docs.google.com/document/d/1JSJTJMJ6ZXLkCSmvOIabTLrjtqqr0E-u99Rx2BHR1hs/edit). > > > Diffs > ----- > > 3rdparty/libprocess/include/Makefile.am > f45e7c5c0fad063cc0b34ec7977cef685c2909d3 > 3rdparty/libprocess/include/process/firewall.hpp PRE-CREATION > 3rdparty/libprocess/include/process/process.hpp > 79d1719932a3fdc90b6247d3a77adee123e72435 > 3rdparty/libprocess/src/process.cpp > d1b4d469a11abc618c1406bce602300dd9793b58 > 3rdparty/libprocess/src/tests/process_tests.cpp > 7b9ba9e70e1fe7a22b26444b3bd928208fecd491 > > Diff: https://reviews.apache.org/r/33295/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
