> On Aug. 27, 2015, 5:54 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 117
> > <https://reviews.apache.org/r/37825/diff/1/?file=1055550#file1055550line117>
> >
> >     I think this function should also parse the data into Map<String, 
> > TierInfo>. The earlier we parse data (and error out if data is invalid) the 
> > better IMHO.

Yeah, addressed above.


> On Aug. 27, 2015, 5:54 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/TierManager.java, line 69
> > <https://reviews.apache.org/r/37825/diff/1/?file=1055551#file1055551line69>
> >
> >     If applying a default value, I think we should log it.

The default tier does not deviate from the current behavior, so logging would 
not add any additional info here. Besides, this will generate quite a bit of 
extra log noise as it's used in the scheduling loop.


> On Aug. 27, 2015, 5:54 p.m., Zameer Manji wrote:
> > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java, line 26
> > <https://reviews.apache.org/r/37825/diff/1/?file=1055552#file1055552line26>
> >
> >     Please add a test for the config for having keys that are not expected. 
> > I would expect the scheduler to reject such files because they are 
> > malformed.

Not sure what you mean by malformed. Are you suggesting adding extra keys or 
purposefully corrupt json file? The former is irrelevant as unknown keys are 
ignored on casting to the type and the latter just throws unhandled parsing 
exception (exactly what we want/expect).


- Maxim


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


On Aug. 27, 2015, 1:07 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37825/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 1:07 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1437
>     https://issues.apache.org/jira/browse/AURORA-1437
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The external config file is optional for now as tiers are not fully defined 
> yet.
> 
> 
> Diffs
> -----
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> d4bc6b9bb10c982fb6a2458d7fdd12bddbe98eca 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 652afece17a7eb09b0ca68066707b1b8fbf024f0 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> eb0207114e1b93968cb65832f154a9cd3bc3232e 
>   src/test/resources/org/apache/aurora/scheduler/tiers-example.json 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37825/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to