.
configure.ac
https://reviews.apache.org/r/33828/#comment133200
Style nit: Any reason you went with `+=` instead of `CXXFLAGS=${CXXFLAGS}
-Wno-maybe-uninitialized` like the rest of the file?
- Adam B
On May 4, 2015, 4:48 p.m., Joris Van Remoortere wrote
On May 11, 2015, 10:58 p.m., Adam B wrote:
src/cli/execute.cpp, lines 203-204
https://reviews.apache.org/r/33109/diff/5-6/?file=939687#file939687line203
You can keep this as one statement, just wrap with the '=' at the start
of the newline.
Correction... wrap after
be so short.
docs/modules.md
https://reviews.apache.org/r/33718/#comment134341
Does it get loaded too by --modules, even if it's not selected by --hooks?
If so, I would say load it instead of introduce it. If not, introduce
seems like a perfect term.
- Adam B
On May 8, 2015, 4:19 p.m
timeouts.
`make check` with new unit tests: ShortPingTimeoutUnreachableMaster and
ShortPingTimeoutUnreachableSlave
Thanks,
Adam B
and
ShortPingTimeoutUnreachableSlave
Thanks,
Adam B
it a consistent
2-space indent.
- Adam B
On June 1, 2015, 8:52 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33295
On June 1, 2015, 1:06 a.m., Adam B wrote:
3rdparty/libprocess/include/process/firewall.hpp, lines 54-55
https://reviews.apache.org/r/33295/diff/6/?file=964946#file964946line54
Please capitalize The beginning of each @param description.
Alexander Rojas wrote:
I do completely
April 10, 2015, 12:25 a.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-2608
https://issues.apache.org/jira/browse/MESOS-2608
Repository: mesos-incubating
Description
---
see summary.
Diffs
-
src/examples/java/TestFramework.java
On May 28, 2015, 10:40 a.m., Adam B wrote:
3rdparty/libprocess/src/help.cpp, line 140
https://reviews.apache.org/r/34655/diff/1/?file=971486#file971486line140
Why does this '/help/' need to be removed?
Marco Massenzio wrote:
because it's wrong - it shouldn't
/#comment138727
It was suggested that we should keep this document shortsweet and only
reference the latest Ubuntu LTS release, and then we can archive the 12.04
instructions in a blog post (on mesos/mesosphere site?).
- Adam B
On June 2, 2015, 11:44 p.m., Marco Massenzio wrote
!
3rdparty/libprocess/include/process/http.hpp (line 473)
https://reviews.apache.org/r/35714/#comment141299
Let's keep these in numerical order, so put this after 404, before 500,
please.
- Adam B
On June 21, 2015, 9:16 a.m., Michael Park wrote
On June 1, 2015, 4:32 p.m., Marco Massenzio wrote:
This is great -sorry it took so long to get to do a review.
Thanks for doing it, I'm quite looking forward to using it to learning more
about the Persistent Framework :)
it would be great if we could have a bit more comments in the
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35506/#review88638
---
Ship it!
Ship It!
- Adam B
On June 16, 2015, 4:24 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35583/#review88637
---
Ship it!
Ship It!
- Adam B
On June 17, 2015, 3:42 p.m., Till
the if/else-if/else
formatting. I could do it before committing, or you could.
- Adam B
On March 8, 2015, 5:54 p.m., Till Toenshoff wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29507/#review86039
---
On May 28, 2015, 4:13 p.m., Adam B wrote
slave failover/shutdown with master using different
--slave_ping_timeout and --max_slave_ping_timeouts.
Ran unit tests with shorter non-default values for ping timeouts.
`make check` with new unit tests: ShortPingTimeoutUnreachableMaster and
ShortPingTimeoutUnreachableSlave
Thanks,
Adam B
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35917/#review89475
---
Ship it!
Ship It!
- Adam B
On June 26, 2015, 12:47 a.m., Bernd
On June 24, 2015, 3:24 a.m., Adam B wrote:
src/common/http.cpp, line 84
https://reviews.apache.org/r/35717/diff/4/?file=989671#file989671line84
I wonder if you could templatize this to work for modeling any
hashmapstring, T, where T is an already modeled type.
I'm not sure what
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35717/#review89456
---
Ship it!
Ship It!
- Adam B
On June 22, 2015, 4:37 a.m
On June 24, 2015, 1:04 a.m., Adam B wrote:
This seems like a positive step forward, since all volumes were previously
owned by root (or whatever user the slave was run as). However, if a
persistent volume is passed from one framework to another within the same
role, they might have
as the slave user, and volumes will need to stay
as that user in order to be accessible by the tasks.
- Adam B
On June 21, 2015, 6:56 p.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35782/#review89141
---
Ship it!
Ship It!
- Adam B
On June 23, 2015, 3:14 a.m
suggested in the
ticket.
haosdent, did you do any (manual) testing on this? Maybe you could link to a
before/after screenshot?
- Adam B
On June 23, 2015, 3:14 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply
twice, but
that's probably not much of a performance hit.
- Adam B
On June 21, 2015, 7:21 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35711
:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35717/
---
(Updated June 22, 2015, 4:37 a.m.)
Review request for mesos, Adam B
--slave_ping_timeout and --max_slave_ping_timeouts.
Ran unit tests with shorter non-default values for ping timeouts.
`make check` with new unit tests: ShortPingTimeoutUnreachableMaster and
ShortPingTimeoutUnreachableSlave
Thanks,
Adam B
.
Thanks,
Adam B
to
`PartitionTest.PartitionedSlave`. For (2), we have
SlaveTest.PingTimeoutNoPings, SlaveTest.PingTimeoutNoPings.
Adam B wrote:
Yes, I see now that SlaveTest.PingTimeoutNoPings already satisfies (1),
and PartitionTest.PartitionedSlave could be modified to check for the
ShutdownMessage
-core or smaller.
- Adam B
On June 2, 2015, 11:44 p.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34976
On June 11, 2015, 3:02 a.m., Adam B wrote:
Sorry it took me so long to get back to this. I just had a few minor
comments/questions, but you ought to get a Mac user to verify the OSX
instructions.
Also, for doc changes like this, it's helpful if you can post a link to a
personal fork
instructions.
docs/getting-started.md
https://reviews.apache.org/r/34976/#comment140107
s/add/append/
docs/getting-started.md
https://reviews.apache.org/r/34976/#comment140108
s/mesos/Mesos/
- Adam B
On June 11, 2015, 11:23 p.m., Marco Massenzio wrote
the Testing section.
ReviewBot says the patch doesn't apply cleanly, so please rebase.
- Adam B
On June 12, 2015, 3:32 a.m., Anand Mazumdar wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35179/#review88312
---
Ship it!
Looks great! I'll get this committed tonight
- Adam B
failover/shutdown with master using different
--slave_ping_timeout and --max_slave_ping_timeouts.
Ran unit tests with shorter non-default values for ping timeouts.
`make check` with new unit tests: ShortPingTimeoutUnreachableMaster and
ShortPingTimeoutUnreachableSlave
Thanks,
Adam B
://reviews.apache.org/r/29507/#review83758
---
On May 14, 2015, 3:01 a.m., Adam B wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r
did you test this? Please fill out the Testing section appropriately.
3rdparty/libprocess/src/help.cpp
https://reviews.apache.org/r/34655/#comment137159
Why does this '/help/' need to be removed?
- Adam B
On May 28, 2015, 12:54 a.m., haosdent huang wrote
://reviews.apache.org/r/34545/#comment137433
`file://path-to-external-allocator.json` is a little confusing,
especially if there are multiple modules that need to be loaded. Maybe just
call it `modules.json` give an example json?
- Adam B
On May 26, 2015, 3:37 a.m., Alexander Rukletsov wrote
On May 28, 2015, 10:40 a.m., Adam B wrote:
Do you clearly understand why this change is needed? I didn't understand
after just reading the JIRA, and had to ask the reporter(s). Mesosphere is
hosting the Mesos UI(s) underneath the DCOS UI behind a reverse proxy, so
that `http
the default id value? All your callers already specify a processId.
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment137781
Would like to see a test for nested endpoints, and maybe even wildcards, so
that it's clear whether or not those work.
- Adam B
.
src/master/http.cpp
https://reviews.apache.org/r/34362/#comment137802
Nit: Please insert blank line between these two like in other similar
blocks.
src/master/http.cpp
https://reviews.apache.org/r/34362/#comment137801
std::move(executors);
- Adam B
On May 18, 2015, 10:41 a.m
On June 1, 2015, 1:34 a.m., Adam B wrote:
src/master/http.cpp, line 1038
https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038
Could I ask you to write a quick unit test for this?
haosdent huang wrote:
yes
haosdent huang wrote:
@adam-mesos, I have
tasks list. Otherwise, if it's
going to return a blank tasks[] array, then it should return something like a
500 (Internal Server Error), or 307 (Temporary Redirect).
- Adam B
On May 24, 2015, 11:22 a.m., haosdent huang wrote
approval.
3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 907 - 910)
https://reviews.apache.org/r/36068/#comment142966
Would be nice if you commented on the meaning of these equality checks,
since they're the heart of this test.
- Adam B
On June 30, 2015, 5:33 p.m., Joris Van Remoortere
On June 1, 2015, 1:34 a.m., Adam B wrote:
src/master/http.cpp, line 1038
https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038
Could I ask you to write a quick unit test for this?
haosdent huang wrote:
yes
haosdent huang wrote:
@adam-mesos, I have
or follow the redirect? if the latter, it might result in duplicate
collection!). Can you send an email to the dev list?
haosdent huang wrote:
Sure.Thank you for your review. @vinodkone
Adam B wrote:
It's important to note that this patch does not redirect on certain
master endpoints
On June 1, 2015, 1:34 a.m., Adam B wrote:
src/master/http.cpp, line 1038
https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038
Could I ask you to write a quick unit test for this?
haosdent huang wrote:
yes
haosdent huang wrote:
@adam-mesos, I have
ticket (MESOS-2636) discusses segfaults within the actual
`getIP()` and `hostname()` methods, so the process would have segfaulted before
reaching the LOG(FATAL). Now that MESOS-2636 has been fixed, we should safely
get back an `ip.error()` that we can actually log.
- Adam B
On June 30, 2015
: 0.22.1
main.cpp:190] Git SHA: d6309f92a7f9af3ab61a878403e3d9c284ea87e0
And if you do include it, I don't think it needs to be inside hostname's
parentheses.
- Adam B
On June 30, 2015, 4:23 p.m., Marco Massenzio wrote
://reviews.apache.org/r/36152/#review90298
---
On July 2, 2015, 4:47 p.m., Adam B wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36152
/containerizer.cpp (lines 1353 - 1356)
https://reviews.apache.org/r/35721/#comment142438
These four lines could easily fit on two. We wrap code at 80 chars.
- Adam B
On June 28, 2015, 7:36 p.m., haosdent huang wrote
` as well?
- Adam B
On June 27, 2015, 4:35 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35711
On June 24, 2015, 6:41 p.m., Ben Mahler wrote:
Actually, we should think about one more thing, how does this interact with
the zookeeper session timeout?
Adam B wrote:
The hardcoded individual ping timeout (15secs) was previously longer than
the default zk session timeout (10secs
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36167/#review90449
---
Ship it!
Ship It!
- Adam B
On July 3, 2015, 7:23 a.m
, categorized.
Diffs (updated)
-
CHANGELOG 252f068
Diff: https://reviews.apache.org/r/36152/diff/
Testing
---
Thanks,
Adam B
---
Thanks,
Adam B
)
https://reviews.apache.org/r/35986/#comment143738
Would love to see this test multiple attributes, so we can be sure that
subsequent colons still work as expected in situations like
`parse(attr1:foo:bar;attr2:baz:qux)`.
- Adam B
On June 28, 2015, 6:49 a.m., haosdent huang wrote
On June 29, 2015, 4:43 p.m., Adam B wrote:
Looks great! I know this is already committed, but I had a few
questions/clarifications. Maybe you've answered these elsewhere, but I've
been out of the loop for a while.
Thanks for answering my questions. I don't think there's anything
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36245/#review90648
---
Ship it!
Ship It!
- Adam B
On July 6, 2015, 10:16 p.m., Joris
/upgrades.md (line 17)
https://reviews.apache.org/r/36010/#comment142552
s/meta data/metadata/g
s/reservation/reservations/
s/object/objects/
Is this a Please do not or a you cannot or you must not? Will it
error? Will things break?
- Adam B
On June 29, 2015, 11:16 a.m., Jie Yu wrote
of the decline API.
docs/upgrades.md (line 15)
https://reviews.apache.org/r/36008/#comment142549
Might also want to mention how (implicitly) declining full/partial offers
plays into this API. Or is there fuller documentation/doxygen you could
reference/link to?
- Adam B
On June 29, 2015, 10
15 - 19)
https://reviews.apache.org/r/36011/#comment142553
We usually order these numerically, rather than grouping them by
feature/theme.
- Adam B
On June 29, 2015, 11:17 a.m., Jie Yu wrote:
---
This is an automatically generated
Description (updated)
---
Added component upgrade order.
Diffs (updated)
-
docs/upgrades.md cef055b
Diff: https://reviews.apache.org/r/36145/diff/
Testing (updated)
---
Reviewers, please let me know if you know of any features that require special
upgrade instructions.
Thanks,
Adam
://reviews.apache.org/r/36145/diff/
Testing
---
Reviewers, please let me know if you know of any features that require special
upgrade instructions.
- What about handling modules during upgrades? Rebuild them before
installing/restarting the other components?
Thanks,
Adam B
/r/36173/#comment143491
Doesn't `( scalar | range | text | , )+` look like a set that doesn't
wrap itself in `{}` braces?
It doesn't look like Attributes::parse handles the , case, so let's
remove that (and the `()+`) from the documentation.
- Adam B
On July 3, 2015, 2:30 p.m., Timothy
31)
https://reviews.apache.org/r/36173/#comment143483
No sets? Or are attributes implicitly sets?
docs/attributes-resources.md (line 39)
https://reviews.apache.org/r/36173/#comment143484
3 types? What about plain text?
- Adam B
On July 3, 2015, 2:30 p.m., Timothy Chen wrote
cherry-picking select patches into 0.23.0-rc2. Does this need
to be one?
If not, create a new `(WIP) Release Notes - Mesos - Version 0.24.0` section
at the top to place your Cleanup ticket under.
- Adam B
On July 7, 2015, 10:59 a.m., Jiang Yan Xu wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36022/#review89797
---
Ship it!
Ship It!
- Adam B
On June 29, 2015, 2:03 p.m., Joris
the right
fix.
src/slave/flags.cpp (line 284)
https://reviews.apache.org/r/36026/#comment142610
s/that //
- Adam B
On June 29, 2015, 3:56 p.m., Brendan Chang wrote:
---
This is an automatically generated e-mail. To reply, visit
On June 25, 2015, 7:30 p.m., Adam B wrote:
Nice work! So, running the command as root first will guarantee that
lt-mesos-executor exists before trying to run the task as the test-user
`nobody`? We might be able to fix this in a cleaner way, but this looks
good enough to me. Just fix
---
On June 26, 2015, 3:12 a.m., Adam B wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29507
as a certain principal in the
request, why do we need to explicitly specify a (potentially different?)
principal in the resources message?
docs/reservation.md (line 309)
https://reviews.apache.org/r/32982/#comment142627
How can there be insufficient resources to unreserve?
- Adam B
On June 27
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36026/#review89820
---
Ship it!
Ship It!
- Adam B
On June 29, 2015, 4:03 p.m., Brendan
in the list must contain a `disk.volume` to be destroyed.
- Adam B
On June 27, 2015, 8:35 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981
://reviews.apache.org/r/36003/#comment142675
s/SSL enabled/SSL-enabled/
s/or not//
- Adam B
On June 29, 2015, 2:50 p.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89839
---
Ship it!
Ship It!
- Adam B
On June 29, 2015, 8:53 a.m., Connor
s/leader master/leading master/g
src/master/master.hpp (line 1096)
https://reviews.apache.org/r/34646/#comment142700
s/Reredirect/Redirect/
s/leader master/leading master/
- Adam B
On June 1, 2015, 8:07 a.m., haosdent huang wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36003/#review89949
---
Ship it!
Ship It!
- Adam B
On June 30, 2015, 11:41 a.m., Joris
with a path.
3rdparty/libprocess/src/process.cpp (lines 2019 - 2020)
https://reviews.apache.org/r/35919/#comment142712
You're not logging the Response body anymore?
- Adam B
On June 29, 2015, 3:21 a.m., Alexander Rojas wrote
/HTTP health check returned/
- Adam B
On July 25, 2015, 11:57 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36816
(line 11)
https://reviews.apache.org/r/36197/#comment147635
s/elected/voted in/ since an election implies a choice of candidates.
- Adam B
On July 28, 2015, 1:11 a.m., Bernd Mathiske wrote:
---
This is an automatically generated e
? Is this
some other upgrade issue?
Is a CHECK too strong of an error check?
Ditto elsewhere.
- Adam B
On Aug. 1, 2015, 12:13 p.m., Kapil Arya wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
On July 27, 2015, 11:36 p.m., Adam B wrote:
Great first patch. Thanks for updating FrameworkInfo on reregistration with
the master too!
A handful of nits in my first pass. I'll take another look once you've
simplified the tests with Kapil's suggestions.
Niklas Nielsen wrote
the master does not know the executor ID for the tasks using
a command executor.?
@bmahler do you have any further thoughts/questions on this simple change?
- Adam B
On Aug. 2, 2015, 12:57 a.m., haosdent huang wrote
/36816/#comment147942
Why is this comment relevant here? You aren't even mentioning https here.
I'd remove it.
include/mesos/mesos.proto (line 207)
https://reviews.apache.org/r/36816/#comment147943
s/compile/is compiled/
- Adam B
On Aug. 4, 2015, 2:28 a.m., haosdent huang wrote
`--with-protobuf=`? All four combinations still need to compile pass the unit
tests.
- Adam B
On July 25, 2015, 1:22 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org
`--with-protobuf=`? All four combinations still need to compile pass the unit
tests.
- Adam B
On July 25, 2015, 1:23 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org
)
https://reviews.apache.org/r/36867/#comment147586
Unused. Remove.
src/tests/master_tests.cpp (line 3595)
https://reviews.apache.org/r/36867/#comment147588
Nit: Only indent 2 spaces after wrapping on `=`.
- Adam B
On July 27, 2015, 6:25 p.m., Neil Conway wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36197/#review93558
---
Ship it!
Ship It!
- Adam B
On July 28, 2015, 11:02 a.m., Bernd
On July 30, 2015, 9:24 a.m., Adam B wrote:
src/slave/slave.cpp, lines 1243-1245
https://reviews.apache.org/r/32587/diff/6/?file=1025122#file1025122line1243
Maybe just warn and leave the CopyFrom there?
Next release, we'll remove the field entirely, and consequently
, @cmaloney
- Adam B
On July 28, 2015, 7:43 p.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36810
remove the `using
namespace` and commit this.
3rdparty/libprocess/src/firewall.cpp (line 22)
https://reviews.apache.org/r/36821/#comment147941
Shouldn't need to have a `using` for the namespace, since the code is all
inside that same namespace.
- Adam B
On July 29, 2015, 10:40 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36820/#review93356
---
Ship it!
Ship It!
- Adam B
On July 26, 2015, 9:51 a.m., Joris
/#comment145527
Can you have multiple resource estimators running on the same node
simultaneously? What about QoS controllers?
- Adam B
On July 14, 2015, 5:08 p.m., Niklas Nielsen wrote:
---
This is an automatically generated e-mail
://reviews.apache.org/r/36547/#comment145870
Why not just check for any 2xx code then? Aren't they all successful in one
way or another?
- Adam B
On July 16, 2015, 9:55 a.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail
useful.
- Adam B
On July 17, 2015, 1 p.m., Niklas Nielsen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36488/
---
(Updated
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39385/#review103950
---
Ship it!
Ship It!
- Adam B
On Oct. 22, 2015, 4:03 p.m., Neil
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39385/#review103951
---
Ship it!
Looks like a legit regexp improvement to me.
- Adam B
with this too.
site/source/blog/2015-10-12-mesos-0-25-0-released.md (line 37)
<https://reviews.apache.org/r/39645/#comment162205>
ReviewBot seems to be complaining about this new blank line. I know it's in
the original from Nik's blog, but let's remove it before committing to git.
-
tps://reviews.apache.org/r/39604/#comment161991>
Nit: Wrap '{' to next line, like all function implementations
- Adam B
On Oct. 23, 2015, 3:21 p.m., Artem Harutyunyan wrote:
>
> ---
> This is an automatically generated e-mail.
1 - 100 of 685 matches
Mail list logo