> 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 wrote: > We can, but this change causes a ripple effect. If we push this into a > custom schema, we have to provide a way of unpacking custom configuration > object into useful mesos data fields or information usefol to the executor. > We need to come to a decision on the best way of doing this if we go for it. > Tangentially, as far as I can tell this is only used when building container > volumes.
As this is greenfield code I'm okay with a bit of a ripple - I'd much rather cluster operators have to write this config file once than change it as our design for it evolves. > On Aug. 26, 2015, 3:27 p.m., Kevin Sweeney wrote: > > examples/vagrant/executors-config-new.json, line 19 > > <https://reviews.apache.org/r/37818/diff/1/?file=1055421#file1055421line19> > > > > It would be ergnomically nice to default this to an empty list if it's > > left unset. > > Renan DelValle wrote: > I'll see if there's a way of doing that. If you come across anything, let > me know. A bit of Googling suggests that setting defaults in a default constructor works with GSON here. In fact it seems that just about any way you can think of doing it GSON supports defaults (confirmed myself here: https://gist.github.com/kevints/d1e26514468863e8c088). Setting defaults in an initializer works too - I suggest following the pattern in `ImmutableDefaultTest`, or `ImmutableInitializeOverrideTest` if you have a `@VisibleForTesting` constructor. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review96605 ----------------------------------------------------------- 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 > >
