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 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
>
>
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)?
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 TIER_CONFIG_FILE = Arg.create(new File(
TierModule.class
.getClassLoader()
.getResource("org/apache/aurora/scheduler/tiers.json")
.getFile()));
```
- Amol
---
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
>
>
