> 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 slipped through the cracks from the previous reviews. > On Aug. 26, 2015, 10:27 p.m., Kevin Sweeney wrote: > > examples/vagrant/executors-config.json, line 1 > > <https://reviews.apache.org/r/37818/diff/1/?file=1055422#file1055422line1> > > > > delete this file? It looks like it has been superceded by the new > > format below This first cut wasn't supposed to include the new schema. I was gonna implement it in the second patch but I guess now is as good a time as any to get this in. > On Aug. 26, 2015, 10: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. I'll see if there's a way of doing that. If you come across anything, let me know. > On Aug. 26, 2015, 10: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? 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. > On Aug. 26, 2015, 10:27 p.m., Kevin Sweeney wrote: > > examples/vagrant/executors-config-new.json, lines 13-15 > > <https://reviews.apache.org/r/37818/diff/1/?file=1055421#file1055421line13> > > > > 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. As per the mesos documentation, all options but the string value are optional. What isn't very clear is how the extract is true by default if it is optional. (source: https://github.com/apache/mesos/blob/master/docs/fetcher.md) - Renan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37818/#review96605 ----------------------------------------------------------- On Aug. 26, 2015, 10: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, 10: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 > >
