> On Aug. 9, 2014, 12:48 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 40
> > <https://reviews.apache.org/r/24521/diff/2/?file=656929#file656929line40>
> >
> >     Drive-by partial review, but this seems like an API regression.  Do you 
> > expect duplicates in the result?
> 
> Maxim Khutornenko wrote:
>     This is the only way to preserve ordering in the result set. I am happy 
> to revert it if we decide to sort on the client. It's a bit weird to support 
> paging with partial results coming out of order though.
> 
> David McLaughlin wrote:
>     +1 for keeping it a list.
> 
> Bill Farner wrote:
>     We can guarantee order with a Set (ImmutableSet.copyOf does this).  I do 
> think it's fair to force the client to sort if they want to guarantee sorted 
> order though (especially since we don't accept a sort directive).

The order will not be guaranteed with python client when data gets through 
thrift serialization. We should either keep List and document the order by 
lastModifiedTS or revert to Set and drop any guarantees. I am fine either way. 


- Maxim


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


On Aug. 11, 2014, 5:45 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24521/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 5:45 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/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/AbstractTBaseTypeHandler.java
>  PRE-CREATION 
>   
> 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/log/LogStorageTest.java 
> 548322be029c6663a75187d9f341e53c5b7ca416 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  cbb9c468bf64691e573762f3734528c7b2e16490 
> 
> 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