Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On July 21, 2014, 2:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 21, 2014, 2:58 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
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Bill Farner]

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

Ship it!


Ship It!

- Bill Farner


On July 21, 2014, 9:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 21, 2014, 9:58 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
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

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

(Updated July 21, 2014, 9:58 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

CR comments.


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 (updated)
-

  
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

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

> On July 2, 2014, 11:11 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 414
> > 
> >
> > 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.
>
> 
> Bill Farner wrote:
> > 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?).

Dropped.


- Maxim


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

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Bill Farner]

> On July 2, 2014, 11:11 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 414
> > 
> >
> > 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
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

> On July 2, 2014, 11:11 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 414
> > 
> >
> > 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.

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.
 


- Maxim


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

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Bill Farner]

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



src/main/thrift/org/apache/aurora/gen/api.thrift


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.


- Bill Farner


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

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > 
> >
> > I much prefer the previous signature.  Why not push the Set->String 
> > translation to the call site?
> 
> Maxim Khutornenko wrote:
> That would require making Veto public, which I did not quite like. 
> Besides, there is only one user of that API and no other use cases that would 
> require a Set. I don't quite like String either but that seemed like a 
> reasonable tradeoff given the current semantic. I am ok reverting it if you 
> think Veto can be made public.
> 
> Bill Farner wrote:
> Yeah, i don't see much harm in making Veto visible.

Done.


> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 486
> > 
> >
> > Doesn't it make more sense to inject this filter into the query?

Sure, done.


> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java, 
> > line 133
> > 
> >
> > The Set->String change rears its head here - since you now need to 
> > match the ordering to satisfy equals().
> 
> Maxim Khutornenko wrote:
>

Reverted.


- Maxim


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


On July 1, 2014, 12:28 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 1, 2014, 12:28 a.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/metadata/NearestFit.java 
> c86f598e2ac226e0a356515b389e76f7c5efb67e 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5d21e5ebe80cda75957475afa9c2b94e202b73b2 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> b60b004adbd6753ec6fef125fd70286be5071c56 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2b9edd41631b48dd729ec1dafcf0cd20808e9c7c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a746c48dd21a401b84ddcc610d7c99b4f35f8135 
> 
> Diff: https://reviews.apache.org/r/23188/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq clean build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

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


Changes
---

CR comments.


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 (updated)
-

  
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

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Bill Farner]

> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > 
> >
> > I much prefer the previous signature.  Why not push the Set->String 
> > translation to the call site?
> 
> Maxim Khutornenko wrote:
> That would require making Veto public, which I did not quite like. 
> Besides, there is only one user of that API and no other use cases that would 
> require a Set. I don't quite like String either but that seemed like a 
> reasonable tradeoff given the current semantic. I am ok reverting it if you 
> think Veto can be made public.

Yeah, i don't see much harm in making Veto visible.


- Bill


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


On July 1, 2014, 12:28 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 1, 2014, 12:28 a.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/metadata/NearestFit.java 
> c86f598e2ac226e0a356515b389e76f7c5efb67e 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5d21e5ebe80cda75957475afa9c2b94e202b73b2 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> b60b004adbd6753ec6fef125fd70286be5071c56 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2b9edd41631b48dd729ec1dafcf0cd20808e9c7c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a746c48dd21a401b84ddcc610d7c99b4f35f8135 
> 
> Diff: https://reviews.apache.org/r/23188/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq clean build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > 
> >
> > I much prefer the previous signature.  Why not push the Set->String 
> > translation to the call site?

That would require making Veto public, which I did not quite like. Besides, 
there is only one user of that API and no other use cases that would require a 
Set. I don't quite like String either but that seemed like a reasonable 
tradeoff given the current semantic. I am ok reverting it if you think Veto can 
be made public.


> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 414
> > 
> >
> > This restricts the API in a way that is asymmetric with the request.  
> > i.e. if i'm given full control with a TaskQuery, why is the response 
> > limited to a single job?

The GetPendingReasonResult has a set of these. It's just an instance of a 
PendingReason that is restricted to a single job.


> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java, 
> > line 133
> > 
> >
> > The Set->String change rears its head here - since you now need to 
> > match the ordering to satisfy equals().

 


- Maxim


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


On July 1, 2014, 12:28 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 1, 2014, 12:28 a.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/metadata/NearestFit.java 
> c86f598e2ac226e0a356515b389e76f7c5efb67e 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5d21e5ebe80cda75957475afa9c2b94e202b73b2 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> b60b004adbd6753ec6fef125fd70286be5071c56 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2b9edd41631b48dd729ec1dafcf0cd20808e9c7c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a746c48dd21a401b84ddcc610d7c99b4f35f8135 
> 
> Diff: https://reviews.apache.org/r/23188/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq clean build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Bill Farner]

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



src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java


I much prefer the previous signature.  Why not push the Set->String 
translation to the call site?



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java


Doesn't it make more sense to inject this filter into the query?



src/main/thrift/org/apache/aurora/gen/api.thrift


This restricts the API in a way that is asymmetric with the request.  i.e. 
if i'm given full control with a TaskQuery, why is the response limited to a 
single job?



src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java


The Set->String change rears its head here - since you now need to match 
the ordering to satisfy equals().


- Bill Farner


On July 1, 2014, 12:28 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23188/
> ---
> 
> (Updated July 1, 2014, 12:28 a.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/metadata/NearestFit.java 
> c86f598e2ac226e0a356515b389e76f7c5efb67e 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  5d21e5ebe80cda75957475afa9c2b94e202b73b2 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> b60b004adbd6753ec6fef125fd70286be5071c56 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2b9edd41631b48dd729ec1dafcf0cd20808e9c7c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a746c48dd21a401b84ddcc610d7c99b4f35f8135 
> 
> Diff: https://reviews.apache.org/r/23188/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq clean build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos in the UI/client.

[Maxim Khutornenko]

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

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/metadata/NearestFit.java 
c86f598e2ac226e0a356515b389e76f7c5efb67e 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
5d21e5ebe80cda75957475afa9c2b94e202b73b2 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
8ee43fa1f0e2e699b0f1a321e673e49221b528ad 
  src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
b60b004adbd6753ec6fef125fd70286be5071c56 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 2b9edd41631b48dd729ec1dafcf0cd20808e9c7c 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
a746c48dd21a401b84ddcc610d7c99b4f35f8135 

Diff: https://reviews.apache.org/r/23188/diff/


Testing
---

gradle -Pq clean build


Thanks,

Maxim Khutornenko