> On Feb. 17, 2015, 10:51 p.m., Kevin Sweeney wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 721
> > <https://reviews.apache.org/r/31136/diff/1/?file=866545#file866545line721>
> >
> >     Should these be marked deprecated now (to be removed in 0.9.0?)

The TODOs added will be addressed as part of AURORA-1093, so yes, but not just 
yet.


> On Feb. 17, 2015, 10:51 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 131
> > <https://reviews.apache.org/r/31136/diff/1/?file=866546#file866546line131>
> >
> >     Consdier adding test coverage for these new branches

I'd like to punt.  This code is a shim just to prove compatibility with 
existing APIs and tests, and will be removed in the next diff as i continue to 
push this change up.


> On Feb. 17, 2015, 10:51 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
> >  line 113
> > <https://reviews.apache.org/r/31136/diff/1/?file=866548#file866548line113>
> >
> >     is this a new convention for parameter names? the below ones use 
> > camelCase.

Looks like a bad find/replace.  Fixed.


> On Feb. 17, 2015, 10:51 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  line 24
> > <https://reviews.apache.org/r/31136/diff/1/?file=866552#file866552line24>
> >
> >     why the underscores here?

Matching existing naming convention in the file.


> On Feb. 17, 2015, 10:51 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 
> > 111
> > <https://reviews.apache.org/r/31136/diff/1/?file=866554#file866554line111>
> >
> >     Is there a good reason to reuse update_ids? Why not aim for 
> >     
> >     UNIQUE(update_id),
> >     UNIQUE(job_key_id, update_id),
> >     ?

I can certainly do that, wasn't sure if reviewers would find it duplicative.  
Nonetheless, the TODO will be addressed before the ticket is resolved, and the 
existing constraint will be changed.


- Bill


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


On Feb. 17, 2015, 10:01 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31136/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2015, 10:01 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1093
>     https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is stage 1 of 2 or 3 to use `IJobUpdateKey` throughout the system to 
> uniquely identify job updates.  This change is localized to the database.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2a77f28b847d4897315e63c62455e49b370ea5fa 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 39f72ccd8a61d7691d587f1597dc705c44d01ee2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java
>  d5dd5a535a017e84c628a740bcc17cc5ad50e5cc 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  028eb7c0c70049c92f1060625b4a5c498a83d845 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.java
>  bbd2f465d68109a8d974830a9a4520f28f92a786 
>   src/main/java/org/apache/aurora/scheduler/storage/db/PruneVictim.java 
> PRE-CREATION 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.xml
>  1bc2a620661d766918e655307f76154488c66c5d 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7fd3a86f636523328ef671b60486fb3f1c3178ec 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml
>  9f23c4b6cdc015465d68c219dfbecf3522f5c551 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 987596f733b7155fbce772e6c74a8095d5da1827 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  be5e7727f782200d633fee1cf17491272431f794 
> 
> Diff: https://reviews.apache.org/r/31136/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to