You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/07/01 02:28:17 UTC
Review Request 23188: Adding getPendingReason RPC to expose scheduling vetos
in the UI/client.
-----------------------------------------------------------
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
Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling
vetos in the UI/client.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > <https://reviews.apache.org/r/23188/diff/1/?file=620711#file620711line89>
> >
> > 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<Veto>. 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
> > <https://reviews.apache.org/r/23188/diff/1/?file=620713#file620713line414>
> >
> > 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
> > <https://reviews.apache.org/r/23188/diff/1/?file=620714#file620714line133>
> >
> > 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.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > <https://reviews.apache.org/r/23188/diff/1/?file=620711#file620711line89>
> >
> > 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<Veto>. 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
> > <https://reviews.apache.org/r/23188/diff/1/?file=620712#file620712line486>
> >
> > 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
> > <https://reviews.apache.org/r/23188/diff/1/?file=620714#file620714line133>
> >
> > 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.
Posted by Bill Farner <wf...@apache.org>.
> On July 1, 2014, 6:14 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 89
> > <https://reviews.apache.org/r/23188/diff/1/?file=620711#file620711line89>
> >
> > 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<Veto>. 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.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/23188/#comment82714>
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
<https://reviews.apache.org/r/23188/#comment82715>
Doesn't it make more sense to inject this filter into the query?
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/23188/#comment82716>
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
<https://reviews.apache.org/r/23188/#comment82718>
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
>
>
Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling
vetos in the UI/client.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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.
>
>
> 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.
Posted by Bill Farner <wf...@apache.org>.
> 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
>
>
Re: Review Request 23188: Adding getPendingReason RPC to expose scheduling
vetos in the UI/client.
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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.
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.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/23188/#comment82892>
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.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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