Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-09-10 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Sept. 11, 2014, 1:24 a.m.) Review request for mesos, Adam B and Benjam

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-09-09 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Sept. 9, 2014, 4:51 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-29 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 29, 2014, 7:53 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-29 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 29, 2014, 5:46 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 28, 2014, 10:10 p.m.) Review request for mesos, Adam B and Benjam

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 28, 2014, 6:11 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Cody Maloney
> On Aug. 28, 2014, 8:37 a.m., Adam B wrote: > > src/common/attributes.cpp, lines 161-164 > > > > > > In your code, this.isSuperset(that) returns false if any of this's > > attributes are not present in that. It there

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
> On Aug. 28, 2014, 5:15 p.m., Tobias Weingartner wrote: > > src/slave/slave.cpp, lines 2736-2738 > > > > > > I appreciate the sentiment here. However, both resources and > > attributes (although more attributes) co

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Tobias Weingartner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51791 --- src/slave/slave.cpp

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
> On Aug. 28, 2014, 8:37 a.m., Adam B wrote: > > Great start! I've got several style nits, and some thoughts about extending > > beyond supersets. > > - How do you plan to test this? Have you tried just restarting a > > checkpointed slave process with new resources/attributes? I think you would

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 28, 2014, 4:42 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 28, 2014, 4:36 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 28, 2014, 4:15 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51753 --- Great start! I've got several style nits, and some thoughts about ex

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51761 --- Patch looks great! Reviews applied: [25111] All tests passed. - M

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
> On Aug. 27, 2014, 8:29 p.m., Timothy Chen wrote: > > src/master/master.hpp, line 1169 > > > > > > I don't think you need a bool either. > > It should either succeed or not, and when it doesn't you throw an erro

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 27, 2014, 9:28 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 27, 2014, 9:10 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Cody Maloney
> On Aug. 27, 2014, 8:29 p.m., Timothy Chen wrote: > > src/master/master.hpp, line 1169 > > > > > > I don't think you need a bool either. > > It should either succeed or not, and when it doesn't you throw an erro

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51687 --- src/master/master.hpp

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
> On Aug. 27, 2014, 7:40 p.m., Timothy Chen wrote: > > src/common/attributes.cpp, line 162 > > > > > > We can't support C++11 yet. We've removed the c++11 auto moved back to "foreach" - Patrick ---

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- (Updated Aug. 27, 2014, 7:48 p.m.) Review request for mesos, Adam B and Benjami

Re: Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51680 --- src/common/attributes.cpp

Review Request 25111: Added the concept of dynamically configurable slave attributes

2014-08-27 Thread Patrick Reilly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/ --- Review request for mesos, Adam B and Benjamin Hindman. Bugs: MESOS-1739 htt