org/r/46203/#comment194479>
Nobody caught that one yet.
- Jan Schlicht
On April 26, 2016, 5:27 p.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing
---
make check
Thanks,
Jan Schlicht
src/tests/slave_authorization_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing
---
make check
Thanks,
Jan Schlicht
> @adam: The three here is needed so that this just strips the agent part
> of the path, not everything up to the last `/`. An example endpoint would be
> `/slave(1)/monitor/statistics`.
>
> Jan Schlicht wrote:
> Seems like a hard problem to fully support both r
://reviews.apache.org/r/46203/diff/
Testing
---
make check
Thanks,
Jan Schlicht
: mesos
Description
---
See summary.
Diffs
-
3rdparty/stout/include/stout/os/read.hpp
c39140fc17c5b4869c3a90c187ebcb9c284397f4
Diff: https://reviews.apache.org/r/47585/diff/
Testing
---
make check
Thanks,
Jan Schlicht
`std::vector` for `buffer`?
I've opened https://reviews.apache.org/r/47585/ to fix this.
3rdparty/stout/include/stout/os/read.hpp (line 142)
<https://reviews.apache.org/r/47481/#comment198581>
s/delete/delete[]
- Jan Schlicht
On May 18, 2016, 5:25 a.m.,
;` instead!
>
> Benjamin Bannier wrote:
> +1, or just a `std::string` since most probably the reflex might be to
> *always* use `Owned` instead of `std::unique_ptr`, but that one cannot be
> used to manage dynamic arrays.
>
> Jan Schlicht wrote:
> We could also use a `s
td::vector buffer;
buffer.reserve(BUFSIZ);
```
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47585/#review133900
-------
> ---
>
> (Updated May 18, 2016, 10 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan
> Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-5317
> https
acd3bdc447e3
> src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052
> src/tests/master_authorization_tests.cpp
> 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8
>
> Diff: https://reviews.apache.org/r/46784/diff/
>
>
> Testing
> ---
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
be622d31de29a242a6c71fd8dedb06aeff19142d
Diff: https://reviews.apache.org/r/47339/diff/
Testing
---
make check
Thanks,
Jan Schlicht
nvenient to make
it static.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47339/#review133075
-------
On May 13,
Diff: https://reviews.apache.org/r/47339/diff/
Testing
---
make check
Thanks,
Jan Schlicht
; > to split endpoint into two parts?
>
> Jan Schlicht wrote:
> Size could be 0 or 1 if the input string doesn't look as expected --
> exactly what we're testing for here.
>
> Alexander Rukletsov wrote:
> Maybe then s/!=/<=/ for documenting our intention?
Yeah, m
/
Testing
---
make check
Thanks,
Jan Schlicht
ave.hpp (lines 96 - 97)
<https://reviews.apache.org/r/47530/#comment198315>
Please don't do this in a header. The `using namespace process;` above is a
bad example and probably shouldn't even be there.
- Jan Schlicht
On May 18, 2016, 12:06 p
://reviews.apache.org/r/46784/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Thanks,
Jan Schlicht
;) Should we
> > just also make this parameterized from the start?
>
> Jan Schlicht wrote:
> I would like to, but unfortunately a parameterized test in GTest needs at
> least two values, at this point we have only one. It doesn't compile with a
> single parameter.
,
Jan Schlicht
---
make check
Thanks,
Jan Schlicht
zeEndpoint()`. Can you share/reuse code between the
> > two?
>
> Jan Schlicht wrote:
> Indeed, that's quite some code duplication. But there are also some
> slight differences in both functions. Had a discussion with @arojas about how
> to handle that case. On one han
n agents! I'll remove it.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46784/#review132450
---
On May 10, 201
zeEndpoint()`. Can you share/reuse code between the
> > two?
>
> Jan Schlicht wrote:
> Indeed, that's quite some code duplication. But there are also some
> slight differences in both functions. Had a discussion with @arojas about how
> to handle that case. On one han
://reviews.apache.org/r/46784/diff/
Testing
---
make check
Thanks,
Jan Schlicht
/master_authorization_tests.cpp
804b39a269c09df9f6c0bbdf6f8b53921ac09ce8
Diff: https://reviews.apache.org/r/46784/diff/
Testing
---
make check
Thanks,
Jan Schlicht
I'd keep it plural here.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46203/#review129083
---
On April 18, 201
e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
---
make check
Thanks,
Jan Schlicht
/slave_authorization_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing
---
make check (SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal fails
though it shouldn't, tests sometimes segfault)
Thanks,
Jan Schlicht
.apache.org/r/45922/#review129097
---
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> --
it's done for all of the other
parameters.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129089
-------
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46318/#review129330
---
Ship it!
Ship It!
- Jan Schlicht
On April 18, 2016, 1:15
b5937af7713e1ee2af475518b3e968b2defa8beb
src/tests/slave_authorization_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing (updated)
---
make check
Thanks,
Jan Schlicht
*limiter, request);
- Jan Schlicht
On April 18, 2016, 1:15 p.m., Benjamin Bannier wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http
1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
---
make check
Thanks,
Jan Schlicht
visit:
https://reviews.apache.org/r/45922/#review128350
-------
On April 14, 2016, 6:16 p.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generat
1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
---
make check
Thanks,
Jan Schlicht
il. To reply, visit:
https://reviews.apache.org/r/45922/#review129694
-------
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generat
/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
---
make check
Thanks,
Jan Schlicht
eviews.apache.org/r/45922/#review129694
-------
On April 21, 2016, 3:45 p.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
>
eed to change these
I need to, because the agent would fail to start otherwise, see
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L439
- Jan
---
This is an automatically generated e-mail. To reply, visi
ttps://reviews.apache.org/r/45922/#review128356
---
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, v
b5937af7713e1ee2af475518b3e968b2defa8beb
src/tests/slave_authorization_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing
---
make check
Thanks,
Jan Schlicht
://reviews.apache.org/r/46203/diff/
Testing
---
make check
Thanks,
Jan Schlicht
/slave_authorization_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/46203/diff/
Testing
---
make check (SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal fails
though it shouldn't, tests sometimes segfault)
Thanks,
Jan Schlicht
into `slave_authorization_tests.cpp` that
was added in https://reviews.apache.org/r/46318/, but that really depends on
whether that change there gets accepted or not.
- Jan Schlicht
On April 18, 2016, 3:42 p.m., Benjamin Bannier wrote
est_filter=*SlaveAuthorization*
Thanks,
Jan Schlicht
different requirements of master and agent. We decided that in this
particular case it's okay to stick with the two similar functions.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/467
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46784/#review132228
---
On May 3, 2016, 2:42 p.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
&g
sit:
https://reviews.apache.org/r/46203/#review132231
---
On April 27, 2016, 4:32 p.m., Jan Schlicht wrote:
>
> ---
> This is an automatically generated e-mail. To reply, v
org/r/47116/ to fix that.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46203/#review131866
---
On April 27, 20
---
See summary.
Diffs
-
src/slave/http.cpp 9b558862e025c5caa71e05fc5eeba783c0ad6fd5
Diff: https://reviews.apache.org/r/47116/diff/
Testing
---
make check
Thanks,
Jan Schlicht
ill add it.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46784/#review132232
---
On May 3, 2016, 2:42 p.m., Jan Schlicht wrote:
>
> ---
&
> On May 7, 2016, 12:24 p.m., Adam B wrote:
> > src/master/http.cpp, line 869
> > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line869>
> >
> > Did you need to pass all ("=") the in-scope variables through to the
> > l
Here
> > you call `master->flags` in the continuation. Why is the approach different
> > to the one for the agent?
>
> Jan Schlicht wrote:
> See [MESOS-5293](https://issues.apache.org/jira/browse/MESOS-5293).
> tldr; We can do it for the master, but (currently) not for
> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 2872
> > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line2872>
> >
> > Why don't we return `MethodNotAllowed` here? Same applies to the agent,
&
> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote:
> > src/master/http.cpp, line 868
> > <https://reviews.apache.org/r/46784/diff/1/?file=1364766#file1364766line868>
> >
> > Could you make this capture list explicit (`[this, request]`)?
>
> Ja
> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 2872
> > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line2872>
> >
> > Why don't we return `MethodNotAllowed` here? Same applies to the agent,
&
ache.org/r/47116/diff/
Testing
---
make check
Thanks,
Jan Schlicht
> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 2872
> > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line2872>
> >
> > Why don't we return `MethodNotAllowed` here? Same applies to the agent,
&
> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 2872
> > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line2872>
> >
> > Why don't we return `MethodNotAllowed` here? Same applies to the agent,
&
ttp.cpp (line 777)
<https://reviews.apache.org/r/47822/#comment199642>
s/pid/slave->self()/
src/slave/http.cpp (line 783)
<https://reviews.apache.org/r/47822/#comment199646>
s/localSlave/slave/
> On May 25, 2016, 11:45 a.m., Jan Schlicht wrote:
> > src/slave/http.cpp, line 651
> > <https://reviews.apache.org/r/47822/diff/1/?file=1393299#file1393299line651>
> >
> > Remove this, same rationale as the comments in `Slave::Http::flags`.
>
> Abh
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56139/#review163951
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 1, 2017, 1 a.m
> On Jan. 30, 2017, 10:24 a.m., Jan Schlicht wrote:
> > src/tests/slave_validation_tests.cpp, line 282
> > <https://reviews.apache.org/r/56055/diff/1/?file=1618348#file1618348line282>
> >
> > Indent with 4 spaces.
>
> Greg Mann wrote:
>
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56187/#review163952
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 1, 2017, 10:26 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56188/#review163953
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 1, 2017, 10:26 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56052/#review163950
---
Ship it!
Ship It!
- Jan Schlicht
On Jan. 31, 2017, 11:44
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55954/#review163128
---
Ship it!
- Jan Schlicht
On Jan. 26, 2017, 3:36 a.m., Greg
tps://reviews.apache.org/r/56055/#comment234960>
Indent with 4 spaces.
- Jan Schlicht
On Jan. 29, 2017, 12:39 a.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55955/#review163484
---
Ship it!
Ship It!
- Jan Schlicht
On Jan. 29, 2017, 12:35
://reviews.apache.org/r/56667/diff/
Testing
---
make check
Thanks,
Jan Schlicht
compliance,
which would mean (among others) support for `alg=none`, probably `alg=RS256`
and other subtleties.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56667/#review166057
c2f64a91a505675d568ddf5aa081125e4e32fe17
3rdparty/libprocess/src/ssl/utilities.cpp
8aec613312eee3dd11d9df8c3828a5185407e073
Diff: https://reviews.apache.org/r/5/diff/
Testing
---
make check
Thanks,
Jan Schlicht
3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION
3rdparty/libprocess/src/jwt.cpp PRE-CREATION
3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/56667/diff/
Testing
---
make check
Thanks,
Jan Schlicht
/r/56753/diff/
Testing
---
make check
Thanks,
Jan Schlicht
described in RFC 4648.
Diffs (updated)
-
3rdparty/stout/include/stout/base64.hpp
2ac04c4602bc919633a2a480dd2168b7aa301bd7
3rdparty/stout/tests/base64_tests.cpp
32e516861d44c7e3a36e1a29b4d1fe5960684e3b
Diff: https://reviews.apache.org/r/56665/diff/
Testing
---
make check
Th
/module/secret_generator.hpp PRE-CREATION
src/Makefile.am c21a073bdbefbb4547e45ed13b7a8a563854fd82
Diff: https://reviews.apache.org/r/56757/diff/
Testing
---
make check
Thanks,
Jan Schlicht
check
Thanks,
Jan Schlicht
> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote:
> > Regarding the description: I'm curious how exactly the current
> > implementation isn't compliant with RFCs 7515/7519? The one thing I noticed
> > was the lack of support for the 'crit' header parameter.
&g
<https://reviews.apache.org/r/56619/#comment237349>
Check for `context.isSome()` first.
src/master/weights_handler.cpp (line 369)
<https://reviews.apache.org/r/56619/#comment237350>
Remove `<< context.get()` here.
- Jan Schlicht
On Feb. 14, 2017, 1:
nature this way:
```
authorization::Subject subject = createAuthorizationSubject(context)
```
What do you think?
- Jan Schlicht
On Feb. 14, 2017, 12:46 a.m., Greg Mann wrote:
>
> ---
> This is an automatically ge
/http.hpp (line 80)
<https://reviews.apache.org/r/56478/#comment236749>
s/authenticator/authenticators/
3rdparty/libprocess/src/authenticator_manager.hpp (line 50)
<https://reviews.apache.org/r/56478/#comment236748>
s/authenticator/authenticators/
- Jan Schlicht
On Feb. 9
tps://reviews.apache.org/r/56476/#comment236750>
s/name/httpAuthenticatorName/
Just `name` feels too general.
src/common/http.cpp (line 956)
<https://reviews.apache.org/r/56476/#comment236751>
Not yours, but please indent with 4 spaces.
- Jan Schlicht
On Feb. 9, 2017, 2:0
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56479/#review164923
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 9, 2017, 7:11 a.m
168 - 204)
<https://reviews.apache.org/r/56474/#comment236756>
How about we make this a function instead of using a lambda?
Seems to do a lot and even contains another lambda in line 180.
- Jan Schlicht
On Feb. 9, 2017, 1:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56475/#review164929
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 9, 2017, 2:03 a.m
/secret_generator.hpp PRE-CREATION
src/authentication/executor/secret_generator.cpp PRE-CREATION
src/tests/secret_generator_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/56754/diff/
Testing
---
make check
Thanks,
Jan Schlicht
cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7
3rdparty/libprocess/src/jwt_authenticator.cpp PRE-CREATION
3rdparty/libprocess/src/tests/http_tests.cpp
fb4da9aecff0370d97a15269c5d8fffb30e0478f
Diff: https://reviews.apache.org/r/56753/diff/
Testing
---
make check
Thanks,
Jan Schlicht
org/r/56052/#comment237676>
Please include ``.
- Jan Schlicht
On Feb. 15, 2017, 11:48 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
Thanks,
Jan Schlicht
s (updated)
-
3rdparty/stout/include/stout/base64.hpp
2ac04c4602bc919633a2a480dd2168b7aa301bd7
3rdparty/stout/tests/base64_tests.cpp
32e516861d44c7e3a36e1a29b4d1fe5960684e3b
Diff: https://reviews.apache.org/r/56665/diff/
Testing (updated)
---
make check
Thanks,
Jan Schlicht
Thanks,
Jan Schlicht
> On Feb. 14, 2017, 4:12 p.m., Jan Schlicht wrote:
> > src/common/http.hpp, line 133
> > <https://reviews.apache.org/r/56618/diff/1/?file=1632574#file1632574line133>
> >
> > How about using an `Option` here and returning
> > `Subject()` in the case o
src/Makefile.am c21a073bdbefbb4547e45ed13b7a8a563854fd82
Diff: https://reviews.apache.org/r/56757/diff/
Testing
---
make check
Thanks,
Jan Schlicht
/56754/diff/
Testing
---
make check
Thanks,
Jan Schlicht
)
<https://reviews.apache.org/r/56623/#comment237688>
Make this a const function.
You could also make this a free function, to have better ordering.
- Jan Schlicht
On Feb. 14, 2017, 12:23 a.m., Greg Mann
7>
You can use the `AuthenticationContext(credential[0])` construtor and
remove the next line.
3rdparty/libprocess/src/authenticator_manager.cpp (line 95)
<https://reviews.apache.org/r/56623/#comment237345>
Please align the `?` in this line with the following ones.
- Jan Schlicht
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56617/#review165700
---
Ship it!
Ship It!
- Jan Schlicht
On Feb. 14, 2017, 4:41 a.m
301 - 400 of 927 matches
Mail list logo