> On Feb. 26, 2016, 4:57 p.m., Bill Farner wrote:
> > Is there a reason this needs to be required as opposed to providing a sane 
> > default (preferably matching current behavior)?
> 
> Amol Deshmukh wrote:
>     You are right, at the moment it would be possible to provide a default 
> value without loss of functionality.
>     
>     The rationale was that, since we would want to make this "required" 
> eventually, it would be easier for cluster operators to deal with this now 
> rather than being struck with the full-complexity of tier management later. I 
> have to admit though, I have been on the fence on this.
>     
>     If making this a required option does not appear sufficiently justified, 
> let me know if providing a default like below seems like a reasonable 
> alternative:
>     ```
>     # Provide Arg-default pointing to the default tiers.json file in 
> classpath.
>     # This will involve moving tiers.json from src/test/... to src/main/...
>     
>     private static final Arg<File> TIER_CONFIG_FILE = Arg.create(new File(
>         TierModule.class
>             .getClassLoader()
>             .getResource("org/apache/aurora/scheduler/tiers.json")
>             .getFile()));
>     ```

I suggest make that the effective default, but not via `Arg.create(...)` just 
because it's unlikely to present well in help output.  Just handle empty/null 
when you read the `TIER_CONFIG_FILE` value.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43925/#review121023
-----------------------------------------------------------


On Feb. 26, 2016, 2:20 p.m., Amol Deshmukh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43925/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 2:20 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1616: [part 2] make tier_config mandatory argument when starting up 
> the scheduler.
> 
> 
> Diffs
> -----
> 
>   NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 45ab76b9331a79699979c6386c93bbc763f64e2e 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> dc1ef82bce9e8e243974f8b97165f4417d870a7e 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> fce6e51548b23b7bc3e33468c2b3a9627a68debd 
>   src/main/java/org/apache/aurora/scheduler/TierModule.java 
> b5f065ec433b4df50a5c1ca7ef87d51292816db6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4c64a1c4bd8596a528f6dabd6f9a794348ded7d8 
>   src/test/java/org/apache/aurora/scheduler/SchedulerModuleTest.java 
> aa6e0350caa6ebe79c46e28e8d7fd7fd8d6c63d4 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> 4da829f7b3aad18b9ed3a390eaa89afcb2f3cd29 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 7ee31fd4a59014e97a36e30b5a6b66f54114ef62 
>   src/test/resources/org/apache/aurora/scheduler/tiers.json 
> 21407738fafc9bc5e6ce7888b4b9c32b2f005bca 
> 
> Diff: https://reviews.apache.org/r/43925/diff/
> 
> 
> Testing
> -------
> 
> # Java build with checkstyle:
> ```
> $ ./gradlew build -Pq
> ...
> BUILD SUCCESSFUL
> 
> Total time: 3 mins 44.365 secs
> ```
> 
> # End-to-end tests:
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> 
> ```
> 
> # Ensure affected benchmarks run:
> ```
> $ ./gradlew -q jmh  -Pbenchmarks='S.*Benchmark.?'
> ...
> Benchmark report generated: file:///.../aurora/dist/reports/jmh/human.txt
> 
> $ ls -1 dist/reports/jmh/human.txt
> dist/reports/jmh/human.txt
> ...
> 
> ```
> 
> # Python client tests:
> ```
> ./pants test.pytest --no-fast src/test/python/apache/aurora::
> ...
>                SUCCESS
> ```
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Reply via email to