> On Jan. 3, 2014, 4:55 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/state/LockManager.java, line 65 > > <https://reviews.apache.org/r/16629/diff/1/?file=415149#file415149line65> > > > > Is there a reason to take an Optional here? Shouldn't the caller just > > not call in if the provided lock is null? > > Maxim Khutornenko wrote: > This is a convenience method for cases like getQuota() where Lock > presence is optional as it's used in both locked (update, create) and > unlocked (getQuota api) contexts. Having Optional here avoids IF condition on > the SchedulerThriftInterface side.
I personally prefer that input sanitization happen closer to the request processing code when possible (which allows us to assume that no nulls are passed to core objects) but I'll defer to consensus here. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16629/#review31197 ----------------------------------------------------------- On Jan. 3, 2014, 4:03 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16629/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2014, 4:03 p.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Repository: aurora > > > Description > ------- > > Part 2: Server side changes for the client quota check. > > Note: api.thrift changes are duplicated from part 1: > https://reviews.apache.org/r/16444/ and will race to submission with it. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/state/LockManager.java > 5f34de1bd0f90269642ea8d451ce4cd6fe180c2d > src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java > bf0a650ae55eaa66ae7ee2b8a201cb5bda38177f > > src/main/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java > 8fb51d69be6d370f9f010c797b2c1205b38a04f5 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > c1a11bdb91c5e764864324d26248d1783af8048b > src/main/thrift/org/apache/aurora/gen/api.thrift > 480b8f472bcfbe547a91c41638250350a0110334 > src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java > c8ad55d8d48f7e96180846ab515dd4df3d8ed79e > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 91c1c24448092e1b3454844ab8074ed030383594 > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java > 62fc8045f6a5fda234df73452685bd04e3142aaf > > Diff: https://reviews.apache.org/r/16629/diff/ > > > Testing > ------- > > gradle build > > > Thanks, > > Maxim Khutornenko > >
