> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote:
> > examples/vagrant/executors-config.json, lines 4-7
> > <https://reviews.apache.org/r/37818/diff/3/?file=1061919#file1061919line4>
> >
> > The code later converts this array into a single command string. I
> > suggest we just make this a string here (and save some code down the line).
>
> Renan DelValle wrote:
> We can go ahead and do that. Kevin thought it should be the array way,
> but I'm not sure how strongly he feels about it. The extra code isn't too bad
> because we can use String.join() form Java 8. Either way, I'm fine with
> either. The single string may eliminate any suspicions that we're breaking
> something in a command when joining the array.
Bill, I prefer the array mode as it's more explicit for dealing with magic
characters in shell commands (like " (){}$").
You can still explicitly do
```
["sh", "-c", "PATH=/opt/aurora/executor/bin:$PATH thermos"]
```
When you prefer the old form, but this form will not automatically expand
environment variables or do other shell magic for you.
Renan - can you update CommandUtil to use the `arguments`[1] field instead of
`shell` and `value` (or drop a TODO for a later patch)? Changing back to
`String.join` defeats part of the purpose of the array form here.
[1]
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L408-L423
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37818/#review97468
-----------------------------------------------------------
On Sept. 1, 2015, 7:35 p.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> -----------------------------------------------------------
>
> (Updated Sept. 1, 2015, 7:35 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.json PRE-CREATION
> examples/vagrant/upstart/aurora-scheduler-kerberos.conf
> 4f43892723db4744db205ea7dd107e9e9ce9d5db
> examples/vagrant/upstart/aurora-scheduler.conf
> e909451892f117e9e6eb80994079661827a0914c
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
> c210c0db07bb1f4b3f76668178dcd7e2de56a4ac
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java
> 197184b6edc0768d677636341b5737f262abdf7d
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
> 8047622e206c9827e5cd8e40152a278d495bd0ff
> src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
> aa5ce8b2f14c7dbd0eae120018ee41387c26059f
>
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
> b3c913892248e4a9a8111412307463985f5ca97f
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
> f6ba2c40aea555d3e0ab774218bfe08d7e1c984b
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java
> 6fad3344042dc6a75cdf74ce79d388fcd4fc9861
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
> 1a25924d789295c5950947f5e302e1d1fbec68f2
> src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java
> cd0295780d41bc4e914583f195b37eaed28a46dc
>
> src/test/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoaderTest.java
> PRE-CREATION
>
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
> dddf7952d3f0e508cd736d5fb95e573267708d43
> src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java
> d0987251b058988fcbfab16c1b138c37e0c5b8c6
>
> src/test/resources/org/apache/aurora/scheduler/configuration/executor-settings-thermos-no-observer.json
> PRE-CREATION
>
> src/test/resources/org/apache/aurora/scheduler/configuration/multiple-executor-example.json
> PRE-CREATION
>
> src/test/resources/org/apache/aurora/scheduler/configuration/no-value-URI.json
> PRE-CREATION
>
> src/test/resources/org/apache/aurora/scheduler/configuration/single-executor-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
>
>
