Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============2198973199353290362=="
MIME-Version: 1.0
Subject: Re: Review Request 23188: Adding getPendingReason RPC to expose
 scheduling
 vetos in the UI/client.
From: "Bill Farner" <wfarner@apache.org>
To: "Bill Farner" <wfarner@apache.org>, "Kevin Sweeney" <kevints@apache.org>
Cc: "Aurora" <reviews@aurora.incubator.apache.org>,
 "Maxim Khutornenko" <maxim@apache.org>
Date: Mon, 07 Jul 2014 18:07:16 -0000
Message-ID: <20140707180716.17971.69782@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20140702231107.5196.80952@reviews.apache.org>
In-Reply-To: <20140702231107.5196.80952@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

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


> On July 2, 2014, 11:11 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 414
> > <https://reviews.apache.org/r/23188/diff/2/?file=623177#file623177line414>
> >
> >     What's the motivation for including the job key?  I would assume that the caller either already knows this, and if not, they can turn back around and query for the task IDs.
> 
> Maxim Khutornenko wrote:
>     TaskQuery allows pulling tasks from multiple jobs. Without specifying some kind of reference towards the parent job, pending reasons would be useless when pulled by anything other than the JobKey. Either we have to further restrict TaskQuery to require role/env/name fields or provide a reference back in the result. Feels like making this RPC effectively job-scoped is an unnecessary restriction at this point.
>

> Without specifying some kind of reference towards the parent job, pending reasons would be useless when pulled by anything other than the JobKey

I disagree that they would be useless.  Like i mentioned above, the caller will either have context, or they can turn around and query for the tasks to gain context.  Including the job key seems arbitrary (i.e. why that and not the instance id?).


- Bill


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


On July 2, 2014, 11:07 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 11:07 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-377
>     https://issues.apache.org/jira/browse/AURORA-377
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding getPendingReason RPC to expose scheduling vetos in the UI/client.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2549dd33d08dfc6058d985127a3f0c1f3984eaa7 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 2cffa74ba36e2afda3340658d6b1afd6cb50cf2c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java ed24ca058c013dfdec8366522893bd4f83e30a9c 
> 
> Diff: https://reviews.apache.org/r/23188/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq clean build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


--===============2198973199353290362==--