----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55982/#review163347 -----------------------------------------------------------
Ship it! LGTM src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java (line 297) <https://reviews.apache.org/r/55982/#comment234754> This message is now outdated and can be dropped. - Stephan Erb On Jan. 26, 2017, 4:16 p.m., Nicolás Donatucci wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55982/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 4:16 p.m.) > > > Review request for Aurora, Stephan Erb and Zameer Manji. > > > Bugs: AURORA-1707 > https://issues.apache.org/jira/browse/AURORA-1707 > > > Repository: aurora > > > Description > ------- > > As the validations for NumCpus, RamMb and DiskMb happened before the thrift > backfill, those values needed to be set, even though they are deprecated. In > the thrift backfill, if the Resources field is set, then NumCpus, RamMb and > DiskMb are set accordingly. > > So by moving those validations, it is now possible to only set the Resources > field instead of having to set the deprecated fields. As the validations are > moved and not removed, the ckeck for the resource values being greater than 0 > still happens. Furthermore, if the Resources field is set but there is no > Resource for Ram in the set, the thrift backfill will throw an > IllegalArgumentException. > > Some tests were slightly modified because of this, mostly by adding an > unsetResources() operation. This is because as the validations now happen > after the thrift backfill, during the thrift backfill the values in the > deprecated fields are replaced by those in the Resources field (if it is > set). There are also some new tests. > > Related Issue: AURORA-1707 > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > f96cd7a8eba12c286ac4e104a22ae74d8d4108d7 > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > 6d0e9bc6a8040393875d4f0a88e8db9d6926a88b > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > b28cd2489a52041a8e7e53f298fad8d8cd29406f > > Diff: https://reviews.apache.org/r/55982/diff/ > > > Testing > ------- > > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Nicolás Donatucci > >