----------------------------------------------------------- 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 examples/vagrant/executors-config-new.json (lines 13 - 15) <https://reviews.apache.org/r/37818/#comment152193> do these have defaults that we can omit? also I don't think we want to cache in the vagrant environment as we want changes to be reflected in new tasks immediately when developing. examples/vagrant/executors-config-new.json (line 14) <https://reviews.apache.org/r/37818/#comment152195> don't extract this examples/vagrant/executors-config-new.json (line 18) <https://reviews.apache.org/r/37818/#comment152194> this isn't a global property - can it be pushed into a custom configuration object? examples/vagrant/executors-config-new.json (line 19) <https://reviews.apache.org/r/37818/#comment152192> It would be ergnomically nice to default this to an empty list if it's left unset. examples/vagrant/executors-config.json (line 1) <https://reviews.apache.org/r/37818/#comment152191> delete this file? It looks like it has been superceded by the new format below src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java (line 114) <https://reviews.apache.org/r/37818/#comment152196> drop redundant use of `this`, here and below. - Kevin Sweeney On Aug. 26, 2015, 3:19 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37818/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2015, 3:19 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > ------- > > This is the first stage in a series of patches to create support for custom > executors. In an effort to expedite the review process, I have decided to > break down my patch into multiple pieces that when/if commited won't break > the trunk. > > This patch includes the ability to load configuration from a JSON file. A > JSON example file is included in examples/vagrant/executors-config.json > > Command line arguments have been eliminated and moved over to the JSON file. > GSON is leveraged and does most of the work with the aid of a few custom > deserializers that were needed. > > Note that right now a global container mount that does not follow > specification will cause the scheduler to detect the error an exit early. It > is up for discussion if this is the desired behavior or if we should just > ignore said mount. > > > Diffs > ----- > > examples/vagrant/executors-config-new.json PRE-CREATION > examples/vagrant/executors-config.json PRE-CREATION > examples/vagrant/upstart/aurora-scheduler-kerberos.conf > 744b4a35c61e749734e222b3d4cbd296927665aa > examples/vagrant/upstart/aurora-scheduler.conf > 789a3a0315e8530880999432aa9b1e7d0f57d1ff > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java > 0c440b5cd5b939872c1ee05d048bf739bfa977cb > > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java > b3c913892248e4a9a8111412307463985f5ca97f > > src/test/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoaderTest.java > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/executor-settings-thermos-no-observer.json > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/thermos-settings-example.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/37818/diff/ > > > Testing > ------- > > ./build-support/jenkins/build.sh: directory sandbox failed but it may be a > flaky test > bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Renan DelValle > >
