Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-10-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review101247 --- Ship it! Master (33d7e21) is green with this patch.

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-30 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/ --- (Updated Sept. 30, 2015, 9:10 p.m.) Review request for Aurora and Bill Farner.

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review101158 --- Ship it! Master (488f29f) is green with this patch.

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review99873 --- This patch does not apply cleanly on master (5627611), do you need

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/ --- (Updated Sept. 21, 2015, 11:42 p.m.) Review request for Aurora and Bill

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-16 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-16 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-16 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-16 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-14 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-14 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-14 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-08 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, lines 4-7 > > > > > > The code later converts this array into a single command string. I > > suggest we just make this a

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-03 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, lines 4-7 > > > > > > The code later converts this array into a single command string. I > > suggest we just make this a

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Bill Farner
> On Aug. 31, 2015, 7:35 p.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, line 1 > > > > > > Can you expand this example to include the command executor? That's > > likely to be the first

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review97468 --- examples/vagrant/executors-config.json (lines 4 - 7)

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java, > > line 87 > > > > > > This code will be easier to maintain long-term with

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Renan DelValle
> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, lines 17-18 > > > > > > Can you omit this since it's blank? Yep, was just leaving it there for now so I don't forget to

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Kevin Sweeney
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, lines 4-7 > > > > > > The code later converts this array into a single command string. I > > suggest we just make this a

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Bill Farner
> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, lines 4-7 > > > > > > The code later converts this array into a single command string. I > > suggest we just make this a

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-01 Thread Renan DelValle
> On Sept. 1, 2015, 2:35 a.m., Bill Farner wrote: > > examples/vagrant/executors-config.json, line 1 > > > > > > Can you expand this example to include the command executor? That's > > likely to be the first

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-31 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/ --- (Updated Sept. 1, 2015, 12:37 a.m.) Review request for Aurora. Changes

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review97220 --- Ship it! Master (c8e65d3) is green with this patch.

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-27 Thread Kevin Sweeney
On Aug. 26, 2015, 3:27 p.m., Kevin Sweeney wrote: examples/vagrant/executors-config-new.json, line 18 https://reviews.apache.org/r/37818/diff/1/?file=1055421#file1055421line18 this isn't a global property - can it be pushed into a custom configuration object? Renan DelValle

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review96733 --- Ship it! Master (06ddaad) is green with this patch.

Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-26 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/ --- Review request for Aurora. Repository: aurora Description --- This is

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-26 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review96605 --- Quick (incomplete) review on the design of the config object

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-26 Thread Renan DelValle
On Aug. 26, 2015, 10:27 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java, line 114 https://reviews.apache.org/r/37818/diff/1/?file=1055427#file1055427line114 drop redundant use of `this`, here and below. Done, apologies, this one