> On Oct. 13, 2015, 8:51 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java,
> >  line 479
> > <https://reviews.apache.org/r/39143/diff/2/?file=1094290#file1094290line479>
> >
> >     "Diff is not currently supported for cron jobs."
> 
> Maxim Khutornenko wrote:
>     Any particular reason to word it like this? My understanding is that we 
> don't have any plans to support cron job updates and this is a job 
> update-specific API. Am I not getting the point?
> 
> Bill Farner wrote:
>     I feel like the functionality gap to add cron job support is minimal, and 
> it would be useful.  I'm not convinced this API needs to be update-specific 
> (i considered pushing back on the name like i did in the design doc, but 
> didn't feel it would be constructive at this point).  The argument that 
> scoping is used is not all that compelling as a basis for excluding cron jobs 
> IMHO.
> 
> Maxim Khutornenko wrote:
>     I just don't see any benefit for cron jobs here. The main point of this 
> API is the instance update sequence to be attempted if an update is 
> scheduled. The reason we bundle TaskConfigs with the result is just a 
> convenience matter for the client config diff (if needed). If we are not 
> going to support cron jobs in the startJobUpdate API I don't see any reason 
> to expose crons in this API. Cron updates are always a direct replacement. As 
> far as diff functionality goes, all is needed there is getJobs API call with 
> TaskConfig diff on the client.
> 
> Bill Farner wrote:
>     Fair enough for the API, but when it comes to presentation to the user i 
> think it's best to provide a uniform command to do this and the client can do 
> the right thing based on the type of job.

This is that API though, right? :) There will be a separate diff for the client 
changes. I suggest we discuss these details there.


- Maxim


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


On Oct. 9, 2015, 10:43 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39143/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 10:43 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-1515
>     https://issues.apache.org/jira/browse/AURORA-1515
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding the API to return update instructions. Design doc: 
> https://docs.google.com/document/d/1Fc_YhhV7fc4D9Xv6gJzpfooxbK4YWZcvzw6Bd3qVTL8
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f56d7bdc5cdbb2fc84254c41328b01c1367c8343 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> c09130bcd4deae80e475f531e5bc08dac475f0e8 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  4efcd2cd81290e919be5cf1de94c5aeb0851c67e 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 
> a17337fb2df8e6e2a2905e88ff1f1eeb03e7d406 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> adcc02b3313220c728a9025b9aec787646f71058 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  82e7330a83ff366fab81385608c62d92445f5e16 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  83f65a73fc2053b138854be13a3b6c7748f80c2b 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> d25618c19615ee097c1f2c4b6fefc70c565c649c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
> 64d45813d89d5f084aec70cd87aadb01de7f6d41 
> 
> Diff: https://reviews.apache.org/r/39143/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to