Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============6242380235082352256=="
MIME-Version: 1.0
Subject: Re: Review Request 39143: Adding getJobUpdateDiff thrift API.
From: "Maxim Khutornenko" <maxim@apache.org>
To: "Bill Farner" <wfarner@apache.org>,
 "David McLaughlin" <david@dmclaughlin.com>
Cc: "Maxim Khutornenko" <maxim@apache.org>,
 "Aurora" <reviews@aurora.apache.org>
Date: Tue, 13 Oct 2015 21:53:30 -0000
Message-ID: <20151013215330.28635.32241@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Maxim Khutornenko" <noreply@reviews.apache.org>
References: <20151013205157.1509.13650@reviews.apache.org>
In-Reply-To: <20151013205157.1509.13650@reviews.apache.org>
Reply-To: "Maxim Khutornenko" <maxim@apache.org>

--===============6242380235082352256==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> 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.

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.


- 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
> 
>


--===============6242380235082352256==--