---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30034/
---
(Updated Feb. 5, 2015, 8:51 p.m.)
Review request for mesos, Adam B, Benjamin
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30034/
---
(Updated Feb. 5, 2015, 9:43 p.m.)
Review request for mesos, Adam B, Benjamin
GitHub user emret opened a pull request:
https://github.com/apache/mesos/pull/35
Update mesos-architecture.md
Fix escaped chars
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/emret/mesos patch-1
Alternatively you can review and
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30590/#review71366
---
Ship it!
src/master/main.cpp
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30589/#review71390
---
src/master/contender.hpp
On Feb. 6, 2015, 4:01 a.m., Benjamin Hindman wrote:
src/master/main.cpp, line 158
https://reviews.apache.org/r/30590/diff/1/?file=846786#file846786line158
Why do we do determine this here? This should be determined down just
before we do 'if (mechanism.isSome())'. Keep the
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30590/#review71369
---
Ship it!
Missed some comments.
docs/configuration.md
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30034/
---
(Updated Feb. 5, 2015, 9:34 p.m.)
Review request for mesos, Adam B, Benjamin
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30588/#review71398
---
src/etcd/etcd.cpp
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30588/#review71394
---
src/Makefile.am
https://reviews.apache.org/r/30588/#comment117131
On Feb. 5, 2015, 2:40 p.m., Alexander Rukletsov wrote:
src/master/master.cpp, lines 613-615
https://reviews.apache.org/r/29883/diff/5/?file=847364#file847364line613
Shouldn't it be /slaves.json? If yes, please correct in other places as
well.
So this was discussed this with
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29883/#review71205
---
Clean and brief, I like it! Some thoughts below.
On Feb. 4, 2015, 11:47 a.m., Alexander Rukletsov wrote:
Ben, I like the refactor you've done. A special thanks goes for updating
comments for clarity and brevity. Below are my 2ยข on naming.
In general, the naming of executors and executor drivers in our codebase is
a bit ambiguous.
On Feb. 5, 2015, 1:40 p.m., Alexander Rukletsov wrote:
src/master/master.cpp, lines 613-615
https://reviews.apache.org/r/29883/diff/5/?file=847364#file847364line613
Shouldn't it be /slaves.json? If yes, please correct in other places as
well.
Alexander Rojas wrote:
So
On Feb. 4, 2015, 7:02 p.m., Alexander Rukletsov wrote:
src/master/master.cpp, lines 601-603
https://reviews.apache.org/r/30612/diff/2/?file=847632#file847632line601
Shouldn't it be `/frameworks.json`?
Alexander Rojas wrote:
This is a good question. On principle, I agree with
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review71224
---
Patch looks great!
Reviews applied: [30074]
All tests passed.
-
On Feb. 5, 2015, 1:26 a.m., Timothy Chen wrote:
src/slave/constants.hpp, line 58
https://reviews.apache.org/r/30601/diff/2/?file=849041#file849041line58
IMO we should name this _MAX instead of _MAXIMUM to be consistent.
Thanks!
- Ben
On Feb. 5, 2015, 12:18 a.m., Niklas Nielsen wrote:
src/slave/constants.hpp, line 58
https://reviews.apache.org/r/30601/diff/2/?file=849041#file849041line58
We should probably make EXECUTOR_SHUTDOWN_GRACE_PERIOD_MAXIMUM
configurable through slave flags. Let's create a ticket for
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30601/
---
(Updated Feb. 5, 2015, 8:55 p.m.)
Review request for mesos, Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30694/#review71318
---
Patch looks great!
Reviews applied: [30694]
All tests passed.
-
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30601/
---
(Updated Feb. 5, 2015, 9:02 p.m.)
Review request for mesos, Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30601/#review71320
---
Bad patch!
Reviews applied: [30579, 30580]
Failed command:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29925/#review71273
---
src/Makefile.am
https://reviews.apache.org/r/29925/#comment116947
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30037/#review71176
---
Several minor things, but I think it looks really solid otherwise.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
---
(Updated Feb. 5, 2015, 4:12 p.m.)
Review request for mesos, Niklas Nielsen and
On Feb. 5, 2015, 2:55 p.m., Niklas Nielsen wrote:
You added some extra state namespace 'using'. Let's keep this (massive)
patch to the internal change only :)
Just updated the description to reflect why we need those additional changes.
- Kapil
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30295/
---
(Updated Feb. 5, 2015, 5:07 p.m.)
Review request for mesos, Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30194/
---
(Updated Feb. 5, 2015, 10:30 p.m.)
Review request for mesos, Benjamin Hindman
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27531/#review71313
---
src/master/master.cpp
On Feb. 5, 2015, 7:38 p.m., Ben Mahler wrote:
src/slave/slave.cpp, lines 1335-1353
https://reviews.apache.org/r/28809/diff/4/?file=848405#file848405line1335
Is it safe to do this once on the combined resources?
```
Resources resources = task.resources();
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30295/#review71326
---
Ship it!
Ship It!
- Niklas Nielsen
On Feb. 5, 2015, 2:07 p.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30195/
---
(Updated Feb. 5, 2015, 10:31 p.m.)
Review request for mesos, Benjamin Hindman
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30195/
---
(Updated Feb. 5, 2015, 10:30 p.m.)
Review request for mesos, Benjamin Hindman
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30300/#review71327
---
Ship it!
Ship It!
- Niklas Nielsen
On Feb. 4, 2015, 7:15 p.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28809/
---
(Updated Feb. 6, 2015, 12:05 a.m.)
Review request for mesos and Ben Mahler.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30694/#review71333
---
src/master/master.cpp
On Feb. 5, 2015, 4:53 p.m., Alexander Rojas wrote:
include/mesos/resources.hpp, lines 301-309
https://reviews.apache.org/r/30654/diff/1/?file=850342#file850342line301
I'm not sure operator overload is the way to go here. I think it will
make the code confusing. Example:
On Feb. 5, 2015, 2:40 p.m., Alexander Rukletsov wrote:
src/master/http.cpp, line 382
https://reviews.apache.org/r/29883/diff/5/?file=847362#file847362line382
Can there be any security issues with exposing slave's PID? Any reason
we want to expose it?
Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30036/#review71228
---
Ship it!
include/mesos/mesos.proto
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30033/#review71225
---
Ship it!
Ship It!
- Benjamin Hindman
On Jan. 19, 2015, 3:45
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30654/#review71233
---
include/mesos/resources.hpp
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30034/#review71227
---
Ship it!
Ship It!
- Benjamin Hindman
On Jan. 20, 2015, 8:33
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30039/#review71181
---
First pass, looks pretty good. Several minor suggestions, but it's
On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
How are we going to manage the duplication across the command executor and
the docker executor?
Timothy Chen wrote:
I think I'm going to leave them seperate as they most likely will grow
independent in tangent.
The docker
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30682/
---
Review request for mesos and Vinod Kone.
Repository: mesos
Description
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30678/#review71259
---
Patch looks great!
Reviews applied: [30678]
All tests passed.
-
On Feb. 5, 2015, 5:53 p.m., Alexander Rojas wrote:
include/mesos/resources.hpp, lines 301-309
https://reviews.apache.org/r/30654/diff/1/?file=850342#file850342line301
I'm not sure operator overload is the way to go here. I think it will
make the code confusing. Example:
On Jan. 31, 2015, 1:05 a.m., Vinod Kone wrote:
src/tests/mesos.hpp, line 676
https://reviews.apache.org/r/29890/diff/7/?file=841772#file841772line676
why does T have to be a HierarchicalDRFAllocator instead of an
Allocator or MesosAllocator?
Alexander Rukletsov wrote:
We
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30678/
---
Review request for mesos and Vinod Kone.
Repository: mesos
Description
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27531/
---
(Updated Feb. 5, 2015, 10:07 a.m.)
Review request for mesos and Vinod Kone.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30678/#review71253
---
Ship it!
Ship It!
- Vinod Kone
On Feb. 5, 2015, 5:47 p.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30682/#review71260
---
Ship it!
Ship It!
- Vinod Kone
On Feb. 5, 2015, 6:30 p.m., Jie
On Feb. 5, 2015, 12:07 a.m., Niklas Nielsen wrote:
src/slave/containerizer/containerizer.cpp, line 237
https://reviews.apache.org/r/30583/diff/2/?file=849006#file849006line237
Wouldn't it simplify executorEnvironment() if we set the default grace
period in getExecutorInfo() (and
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30295/#review71287
---
You added some extra state namespace 'using'. Let's keep this
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29929/#review71293
---
empty src/master/allocation/mesos/hierarchical.hpp ?
- Vinod Kone
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29931/#review71296
---
empty hierarchical.hpp?
- Vinod Kone
On Feb. 4, 2015, 4:53 p.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28809/#review71279
---
Looks good! Will take another look when Vinod's comments are
On Feb. 5, 2015, 12:07 a.m., Niklas Nielsen wrote:
Can you expand on the bug that happened when we didn't pass the slave flags
from CreateSlaveFlags() in the tests?
There is no impacting bug here, but it's an anti-pattern because the slave's
flags are used by the containerizer and so
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29927/#review71292
---
Ship it!
Ship It!
- Vinod Kone
On Feb. 4, 2015, 4:50 p.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29932/#review71302
---
Ship it!
Ship It!
- Vinod Kone
On Feb. 4, 2015, 4:55 p.m.,
On Feb. 4, 2015, 11:55 p.m., Niklas Nielsen wrote:
src/slave/graceful_shutdown.cpp, line 64
https://reviews.apache.org/r/30580/diff/1/?file=846976#file846976line64
Maybe this should be a Try in the future :)
One step at a time.. :)
First thing is we need a check in place for the
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30082/#review71301
---
i'll review this once the directory hierararchy is ironed out.
-
On Feb. 4, 2015, 4:07 p.m., Niklas Nielsen wrote:
src/slave/containerizer/containerizer.cpp, line 237
https://reviews.apache.org/r/30583/diff/2/?file=849006#file849006line237
Wouldn't it simplify executorEnvironment() if we set the default grace
period in getExecutorInfo() (and
On Feb. 5, 2015, 12:18 a.m., Niklas Nielsen wrote:
include/mesos/mesos.proto, line 256
https://reviews.apache.org/r/30601/diff/2/?file=849039#file849039line256
This is the only place the framework writer sees grace period related
docs. Maybe worthwhile documenting SIGTERM -
64 matches
Mail list logo
