You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2016/03/01 02:18:05 UTC
Re: Review Request 43925: AURORA-1616: [part 2] make tier_config
mandatory argument when starting up the scheduler.
> 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
>
>