rs can't use that flag? Make this a WARN instead
- Adam B
On March 21, 2016, 8:42 a.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://re
> On March 21, 2016, 3:28 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 351-354
> > <https://reviews.apache.org/r/45036/diff/1/?file=1306183#file1306183line351>
> >
> > Looks like `Master::Http::FLAGS_HELP()` is incorrectly using
> > AUTHEN
_tests.cpp (line 1710)
<https://reviews.apache.org/r/45166/#comment187703>
s/at least //
This tests that there are exactly 2.
- Adam B
On March 22, 2016, 10:29 a.m., Anand Mazumdar wrote:
>
> ---
> This is an autom
- 434)
<https://reviews.apache.org/r/44846/#comment187136>
"Path to a JSON-formatted file containing the credential to use to
authenticate with the master."
- Adam B
On March 15, 2016, 7:01 a.m., Jan Schlicht wrote:
>
> ---
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44554/#review124523
---
Ship it!
Ship It!
- Adam B
On March 20, 2016, 11:58 a.m
ntials is specified (and ignored)?
src/slave/slave.cpp (lines 407 - 409)
<https://reviews.apache.org/r/44515/#comment187122>
Should we also EXIT/WARN if `--authenticate_http` is not set, and a custom
authenticator is set: `--http_authenticators != DEFAULT_HTTP_AUTHENTICATOR`?
- Adam B
On Ma
---
On March 20, 2016, 9:50 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
type, and we don't get the unnecessarily verbose
"infos" field in the `GET /weights` output?
- Adam B
On March 8, 2016, 10:36 p.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
a `vector, vector` or maybe just a `WeightInfo
createWeightInfo(string, double)`? Neither require stringify/atof
- Adam B
On March 14, 2016, 6:52 a.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, vi
blank lines, please.
- Adam B
On March 14, 2016, 7:07 a.m., Yongqiao Wang 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/44954/#review124167
---
Ship it!
Ship It!
- Adam B
On March 17, 2016, 10:33 a.m
entication/http/basic_authenticator_factory.hpp (line 66)
<https://reviews.apache.org/r/44703/#comment186564>
s/which contains/containing/
s/the authenticator/a new authenticator/
- Adam B
On March 17, 2016, 12:40 p.m., Greg Mann wrote:
>
> -
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44766/#review124165
---
Ship it!
Ship It!
- Adam B
On March 17, 2016, 10:32 a.m
ication_tests.cpp (line 95)
<https://reviews.apache.org/r/44678/#comment186557>
Any reason you called it `entry` instead of `parameter`?
src/tests/http_authentication_tests.cpp (line 100)
<https://reviews.apache.org/r/44678/#comment186556>
Why the second check? This is
oop
src/tests/resource_offers_tests.cpp (line 94)
<https://reviews.apache.org/r/44989/#comment186579>
Not yours, but ".Times(1)" is redundant as it is the default behavior.
Please remove.
- Adam B
On March 1
-
On March 17, 2016, 4:57 a.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44767/
> ---
>
> (Updated March 1
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44767/#review124166
---
Ship it!
Ship It!
- Adam B
On March 17, 2016, 4:57 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45034/#review124291
---
Ship it!
Ship It!
- Adam B
On March 18, 2016, 8:18 a.m
ave the opportunity to not use a trailing
underscore at all, so take it.
- Adam B
On March 17, 2016, 12:41 p.m., Greg Mann wrote:
>
> ---
> This is an automaticall
guration.md (line 1280)
<https://reviews.apache.org/r/44554/#comment186585>
s/JSON file//
docs/home.md (line 27)
<https://reviews.apache.org/r/44554/#comment186586>
s/Framework// here too
- Adam B
On March 17,
> On March 18, 2016, 1:55 a.m., Adam B wrote:
> > src/authentication/http/basic_authenticator_factory.cpp, lines 87-91
> > <https://reviews.apache.org/r/44678/diff/7/?file=1304582#file1304582line87>
> >
> > Is it ok to specify a realm but no credentials? Doe
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44678/#review124384
---
Ship it!
Ship It!
- Adam B
On March 18, 2016, 10:28 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44523/#review124388
---
Ship it!
Ship It!
- Adam B
On March 17, 2016, 12:42 p.m
che.org/r/44703/#comment186910>
Remove
- Adam B
On March 18, 2016, 11:13 a.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http
tps://reviews.apache.org/r/44989/#comment186913>
Perhaps this is why `CreateSlaveFlags()` used `directory.get()` (mkdtemp)
instead of `os::getcwd()`?
If so, maybe you don't even need this patch, because you introduced the
race in a prior patch?
- Adam B
On March 18, 2016, 11:55 a.m., Gre
> On March 18, 2016, 2:47 a.m., Adam B wrote:
> > src/tests/resource_offers_tests.cpp, line 63
> > <https://reviews.apache.org/r/44989/diff/2/?file=1304722#file1304722line63>
> >
> > Why pause so soon? You can wait until after the master is started, but
(line 177)
<https://reviews.apache.org/r/44515/#comment186911>
Why use `os::getcwd()` instead of `directory.get()` like "credential" and
"fetch" above? Seems like `CreateSlaveFlags` wanted to use a temporary
directory removed later.
I'm not convinced these two
g/r/44553/#comment186582>
`s/__recover/recover/` (for the variable name)
- Adam B
On March 17, 2016, 3:56 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://re
> On March 16, 2016, 3:30 a.m., Adam B wrote:
> > Ship It!
Went back to look at the patch that originally added the Disk column, and found
one more copy/paste error in src/webui/master/static/slave.html
I'm fixing that one too with the same commit.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44888/#review123850
---
Ship it!
Ship It!
- Adam B
On March 16, 2016, 3:05 a.m
think we're good to go.
Anybody else want to take a look before I commit?
include/mesos/v1/mesos.proto (lines 440 - 442)
<https://reviews.apache.org/r/44570/#comment185658>
Update these comments to match the others.
- Adam B
On March 14, 2016, 3:46 a.m., Jan Schlicht
ail. To reply, visit:
> https://reviews.apache.org/r/44570/
> ---
>
> (Updated March 14, 2016, 2:37 a.m.)
>
>
> Review request for mesos, Adam B and Joerg Schad.
>
>
> Bugs: MESOS-4772
> https://issues.apache.org/jira/browse/MESOS-4772
>
>
> Rep
ates that ownership is not set on TaskInfo if
ExecutorInfo is present."
src/tests/master_validation_tests.cpp (lines 1166 - 1167)
<https://reviews.apache.org/r/44570/#comment185656>
Update comment please.
"This test verifies that owner can only be set in TaskInfo if Ex
update, and you
should move your new comment into the hpp now.
- Adam B
On March 13, 2016, 9:18 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
<https://reviews.apache.org/r/44703/#comment185650>
No @param for `credentials`?
- Adam B
On March 13, 2016, 9:17 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://re
ot, so it doesn't fit the "type omitted
on the left is already fully stated on the right" qualification.
- Adam B
On March 13, 2016, 9:02 p.m., Greg Mann wrote:
>
> ---
> This is an automatically
/tests/dynamic_weights_tests.cpp (line 217)
<https://reviews.apache.org/r/44511/#comment185633>
Fix indent
- Adam B
On March 9, 2016, 2:03 a.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To
tps://reviews.apache.org/r/41790/#comment185631>
Fix the indent here
- Adam B
On March 9, 2016, 1:47 a.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
src/master/http.cpp (line 837)
<https://reviews.apache.org/r/44767/#comment185625>
s/"This"/false/
- Adam B
On March 13, 2016, 11:18 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically gener
2>
What if this was a bool instead of a string? How many values do you expect?
- Adam B
On March 13, 2016, 3:31 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To
g/r/44765/#comment185617>
s/of/about/
src/master/http.cpp (line 780)
<https://reviews.apache.org/r/44765/#comment185618>
What about `Slave::Http::FLAGS_HELP()`?
src/master/http.cpp (line 786)
<https://reviews.apache.org/r/44765/#comment185619>
s/provides information
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44764/#review123374
---
Ship it!
Ship It!
- Adam B
On March 13, 2016, 1:18 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44621/#review123373
---
Ship it!
Ship It!
- Adam B
On March 13, 2016, 11:50 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44711/#review123372
---
Ship it!
Ship It!
- Adam B
On March 13, 2016, 11:54 a.m
> On March 13, 2016, 12:26 a.m., Adam B wrote:
> > src/tests/master_tests.cpp, line 4265
> > <https://reviews.apache.org/r/44621/diff/4/?file=1296326#file1296326line4265>
> >
> > There is no GET request allowed on /weights (yet), so it's interesting
>
> On March 14, 2016, 12:50 a.m., Adam B wrote:
> > Looks pretty good to me, but I'd like to get AlexR to take a look over it.
> > Also, do we need to rescind inverse offers here too? I'm guessing not, but
> > I'd like somebody to confirm.
And let's do the t
he.org/r/44450/#comment185603>
s/in case/if/
src/master/weights_handler.cpp (line 161)
<https://reviews.apache.org/r/44450/#comment185604>
s/registered frameworks/a registered framework/
- Adam B
On March 8, 2016, 9:19 p.m., Yongqiao Wang wrote:
>
> --
(lines 457 - 458)
<https://reviews.apache.org/r/41790/#comment185595>
Wrap at `<<` instead
src/tests/dynamic_weights_tests.cpp (lines 563 - 564)
<https://reviews.apache.org/r/41790/#comment185596>
Wrap at `<<` instead
- Adam B
On March 9, 2016, 1:47 a.m., Yongqiao Wang
> On March 13, 2016, 4:31 a.m., Adam B wrote:
> > Ship It!
Sorry, needs a rebase after https://reviews.apache.org/r/44693/
- Adam
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.or
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44584/#review123312
---
Ship it!
Ship It!
- Adam B
On March 10, 2016, 10:40 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44583/#review123311
---
Ship it!
Ship It!
- Adam B
On March 9, 2016, 10:04 a.m
che.org/r/43819/#comment185527>
Looks like this fits on one line now. I wonder if others will too.
- Adam B
On March 7, 2016, 12:14 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44523/#review123306
---
Ship it!
Ship It!
- Adam B
On March 11, 2016, 2:04 a.m
line flag. If the slave and the
executor are running in separate containers, `ContainerInfo.volumes` can be
used to mount SSL files from the host into the executor's container.
- Adam B
On March 11, 2016, 11:11 a.m., Jan Schlicht wrote
--authenticate_http value)
- Adam B
On March 11, 2016, 8:48 a.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
&
adAuthentication/ObserveEndpointBadAuthentication/ since
`HealthTest` sounds too generic to just apply to the observe endpoint.
- Adam B
On March 11, 2016, 11:49 a.m., Joerg Schad wrote:
>
> --
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44186/#review123301
---
Ship it!
Ship It!
- Adam B
On March 11, 2016, 11:12 a.m
the executor is the unit of containerization, ownership, and
access control.
The only reason we need a TaskInfo.owner is because there is no
ExecutorInfo for the default command executor. Same holds true for CommandInfo.
- Adam B
On March 10, 2016, 4:49 a.m., Jan Sc
> On March 9, 2016, 10:31 p.m., Adam B wrote:
> > src/master/validation.cpp, line 381
> > <https://reviews.apache.org/r/44570/diff/3/?file=1293312#file1293312line381>
> >
> > It may be sufficient to only check `if (task.has_owner() &&
> > task.
che.org/r/44320/#comment185009>
re-alphabetize
src/authorizer/authorizer.cpp (line 41)
<https://reviews.apache.org/r/44320/#comment185010>
Double-blank line between impls, please
- Adam B
On March 9, 2016, 6:07 a.m
them with a
bad credential will fail with status Unauthorized? See
`TeardownTest.TeardownEndpointBadCredentials` or
`PersistentVolumeEndpointsTest.BadCredentials`.
- Adam B
On March 9, 2016, 9:59 p.m., Greg Mann wrote
ted e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
>
> (Updated March 9, 2016, 12:42 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
>
>
> Bugs: MESOS-484
3)
<https://reviews.apache.org/r/44515/#comment185006>
s/credentials/http credentials/
- Adam B
On March 9, 2016, 12:47 p.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://r
4991>
Fix your commas,
- Adam B
On March 9, 2016, 11:28 p.m., Neil Conway wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
>
tps://reviews.apache.org/r/44583/#comment184987>
Why is the space removed here?
Do we need to put a space after `JSON` instead?
- Adam B
On March 9, 2016, 10:04 a.m., Neil Conway wrote:
>
> ---
> This is an automatically generated e
ExecutorInfo not the TaskInfo.
src/tests/master_validation_tests.cpp (line 1185)
<https://reviews.apache.org/r/44570/#comment184981>
Also, a task with a set owner and an executor without a set owner is
invalid?
- Adam B
On March 9, 2016, 7:48 a.m.
false and then still createBasicAuthHeaders for these tests?
Why can't we keep this at `true`?
- Adam B
On March 9, 2016, 9:56 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To reply,
we would do that for the operator endpoints?
src/master/master.cpp (line 2826)
<https://reviews.apache.org/r/44322/#comment184807>
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 c
ction what the new/updated
tests are?
- Adam B
On March 8, 2016, 10:34 p.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://revi
, after r/44512
(Support to get weights info by /weights). If you revert to the previous
revision, depending on /roles instead of GET /weights, then I can commit these
tests much sooner, and you can add a patch later to update the tests to use GET
/weights.
- Adam B
On March 8, 2016, 9:52 p.m
bad if `weightsFlag` was an empty string with no
tokens? You'd just return an empty `infos`.
- Adam B
On March 8, 2016, 9:52 p.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To r
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42705/#review122694
---
Ship it!
Ship It!
- Adam B
On March 8, 2016, 8:09 p.m
)
<https://reviews.apache.org/r/43824/#comment184800>
Comparing `int` vs. `unsigned` lead to the ReviewBot error you got. I'd
just stick with `int`, since it's shorter and the compiler will probably
optimize all this out anyway.
- Adam B
On March 8, 2016, 7:17 p.m., Yongqiao Wang
of cleanup, and I'll commit it.
src/tests/persistent_volume_tests.cpp (line 683)
<https://reviews.apache.org/r/44408/#comment184797>
`StartMaster()` without any parameters will call `CreateMasterFlags()`
anyway, so just drop the `CreateMasterFlags()`.
- Adam B
On March 7, 2016, 12:55 p.m.
tps://reviews.apache.org/r/44523/#comment184737>
Would the same hold true for any machine that runs both a master and an
agent process?
Or is it only when master and agent are inside the same linux process?
- Adam B
On March 8, 2016, 11:01 a.m., Greg Mann
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44513/#review122626
---
Ship it!
Ship It!
- Adam B
On March 8, 2016, 7:15 a.m
.
(This patch also needs a minor rebase)
docs/configuration.md (lines 761 - 763)
<https://reviews.apache.org/r/42705/#comment184733>
These same changes need to be made in `src/master/flags.cpp` so they show
up in command-line help.
- Adam B
On Feb. 13, 2016, 7:01 p.m., Yongqiao Wang
ralize the method and reuse it for the 3rd instance.
- Adam B
On March 8, 2016, 4:10 a.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
org/r/43824/#comment184543>
s/due to/because/
src/tests/hierarchical_allocator_tests.cpp (line 2605)
<https://reviews.apache.org/r/43824/#comment184544>
s/,/;/
- Adam B
On March 7, 2016, 5:55 a.m., Yongqiao Wang wrote:
>
> ---
reply, visit:
> https://reviews.apache.org/r/43824/
> ---
>
> (Updated March 7, 2016, 5:55 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4200
> https://issues.apache.org
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review122490
---
Ship it!
Ship It!
- Adam B
On Feb. 27, 2016, 4:43 a.m
istry value instead."
docs/weights.md (lines 22 - 23)
<https://reviews.apache.org/r/42719/#comment184517>
This will have to be updated when we add GET
- Adam B
On March 7, 2016, 3:35 a.m
out renaming this to `validatedWeightInfos`? Both this and
`weightInfos` are for updating, but only one of these is validated.
src/master/weights_handler.cpp (line 111)
<https://reviews.apache.org/r/41681/#comment184504>
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43802/#review122449
---
Ship it!
Ship It!
- Adam B
On Feb. 20, 2016, 5:05 p.m
---
On Feb. 21, 2016, 7 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -------
&
)
<https://reviews.apache.org/r/43838/#comment184195>
Definitely ought to cover the python interface too. See
`src/python/interface/src/mesos/interface/__init__.py:177: def
requestResources(self, requests):`
- Adam B
On Feb. 22, 2016, 8:54 a.m., Joerg Schad
DER_EQ(APPLICATION_JSON,
"Content-Type", masterState);`?
Or is the `ASSERT_SOME(parse)` good enough to indicate that we received
json?
- Adam B
On March 2, 2016, 12:53 p.m., Joerg Schad wrote:
>
> ---
> This is an automatically g
12:36 a.m., Yongqiao Wang wrote:
>
> -------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41672/
> ---
>
> (
> On Feb. 27, 2016, 4:21 a.m., Adam B wrote:
> > Sorry this took me forever to get to. Looks pretty good except for some
> > indentation and some unnecessary lines.
> > I'd also like to see a couple of registrar tests, like AlexR suggested.
>
> Yongqiao Wang
/slave_tests.cpp (line 1161)
<https://reviews.apache.org/r/44063/#comment182787>
Why not verify the failureUpdate is TASK_FAILED?
- Adam B
On Feb. 26, 2016, 10:59 a.m., Benjamin Bannier wrote:
>
> ---
> This is an automatically g
won't call `allocate()` since no framework exists in 'role3'
yet, but `addFramework` will.
- Adam B
On Feb. 21, 2016, 11:23 p.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-
eviews.apache.org/r/41681/#comment182775>
The list of roles whose weights can be updated.
- Adam B
On Feb. 14, 2016, 4:02 a.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
egative/non-positive/ (since 0 is also invalid)
- Adam B
On Feb. 18, 2016, 11:56 p.m., Yongqiao Wang wrote:
>
> ---
> This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44144/#review121127
---
Ship it!
Ship It!
- Adam B
On Feb. 27, 2016, 9:49 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43806/#review121087
---
Ship it!
Ship It!
- Adam B
On Feb. 27, 2016, 4:26 a.m
.
- Adam B
On Feb. 24, 2016, 9:32 p.m., Abhishek Dasgupta wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43716/#review121084
---
Ship it!
Ship It!
- Adam B
On Feb. 24, 2016, 10:51 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43328/#review121082
---
Ship it!
Ship It!
- Adam B
On Feb. 24, 2016, 10:50 a.m
e allowed through without authn.
In this setup, the master would still use its --credentials to authenticate
any client that tries to use credentials.
src/tests/dynamic_weights_tests.cpp (lines 553 - 556)
<https://reviews.apache.org/r/41790
roles for registered frameworks,
but quotaRoleSorter contains any role with quota set, regardless of whether any
frameworks are registered with that role."
This might belong above both the quota and roleSorter checks.
- Adam B
On Feb. 20, 2016, 6:26
301 - 400 of 685 matches
Mail list logo
