> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 62
> > <https://reviews.apache.org/r/24521/diff/6/?file=658774#file658774line62>
> >
> >     just <p>, since p is a non-void HTML tag

Done.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 82
> > <https://reviews.apache.org/r/24521/diff/6/?file=658776#file658776line82>
> >
> >     revert

Done.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java, 
> > line 24
> > <https://reviews.apache.org/r/24521/diff/6/?file=658777#file658777line24>
> >
> >     You might consider building in a gate here and throwing 
> > IllegalStateException if setId() was not called (avoid spread of bad data).

Good idea. Done.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  line 324
> > <https://reviews.apache.org/r/24521/diff/6/?file=658782#file658782line324>
> >
> >     This was clearly intentional, please document your reasoning.

Done.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 600
> > <https://reviews.apache.org/r/24521/diff/6/?file=658784#file658784line600>
> >
> >     Better doc?  Also, this is identical to the doc for JobUpdateSummary, 
> > and they're definitely not the same thing.

Thanks, clarified.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java,
> >  line 63
> > <https://reviews.apache.org/r/24521/diff/6/?file=658785#file658785line63>
> >
> >     This feels like a holdover from the previous diff.  How about 
> > s/DEFAULT/FIRST/, and s/INIT/ROLLING_FORWARD/?

Modified and dropped INIT status from JobUpdateStatus enum.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java,
> >  line 119
> > <https://reviews.apache.org/r/24521/diff/6/?file=658785#file658785line119>
> >
> >     Doesn't this duplicate the test case above?

Not really. The above saves with empty set and this one saves with a null set. 
In both cases the result on select is an empty set.


> On Aug. 12, 2014, 9:11 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java,
> >  line 148
> > <https://reviews.apache.org/r/24521/diff/6/?file=658785#file658785line148>
> >
> >     This makes for a nicer error message on failures:
> >     
> >     assertEquals(ImmutableList.of(DEFAULT_EVENT), getUpdateDetails(...));

Changed.


- Maxim


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


On Aug. 12, 2014, 8:26 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24521/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2014, 8:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-612
>     https://issues.apache.org/jira/browse/AURORA-612
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding support for storing task configs and updateOnlyTheseInstances option.
> Implementing the rest of defined JobUpdateStore APIs.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> a9a325b877898be8a265c45112757deba0c3583f 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 808595817c31f3ef3515b623bbeb575bbf1f73fe 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 87f428be892204b8149170d39d3ace2962abc4bd 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2a75646867a5af246625f7cf3910fd9a1bbf4f03 
>   src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  5b7a3df035e78832ba4e6b595202d06af5d664a5 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TaskConfigTypeHandler.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
>  c5468b16709e7bc4758a0597bbe14257b31686ab 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml 
> ce4912d70fe75ea97f3d1bfb7fc4d5ec437fc83b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  3c69bda9fcb473205a3e308b15ad18cd1ae3cba7 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> f0c8336cde4d262bcaf99921c556875ba819b05c 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 4ea9ec2b96fc12429f33336b53e677e12662ec9a 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  c76ab5cbde907a352dce25da585951831733487d 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
>  4c8238955464960e14ab858dcff7a53a64fcd72a 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
> 7ed6691f497ebaaaeb43d522af1358cc0b9491af 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbQuotaStoreTest.java 
> 1f0af8669b655f773faa13a69887c6b0eb57dc57 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbSchedulerStoreTest.java
>  64e2fee337be597f6af4ffaa356872d34d316530 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 548322be029c6663a75187d9f341e53c5b7ca416 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  cbb9c468bf64691e573762f3734528c7b2e16490 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> 6bc6ccf37a34bfe6baf260034584be758a4c83f5 
> 
> Diff: https://reviews.apache.org/r/24521/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> ./pants src/test/python/apache/aurora/client/api:scheduler_client
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to