You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Suman Karumuri <ma...@apache.org> on 2014/01/30 23:27:04 UTC
Review Request 17562: Added cluster name to page title.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Bill Farner.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> >
>
> Suman Karumuri wrote:
> Spoke to Bill before making these changes. We decided to use getInfo API and deprecate getAPIVersion. The getInfo will return with the clusterName and APIVersion for now. In future it may include other attributes and hence the flexible map.
>
> Thanks for the code comments. Will incorporate them once the API changes are confirmed.
>
> Kevin Sweeney wrote:
> For this case the magic name still doesn't make sense to me as it's defined in api.thrift. Why not just make it a field and maintain type-safety?
>
> Bill Farner wrote:
> I agree with Kevin, for all intents and purposes you can consider the thrift struct a map. Formalizing this with field names gives a more consumable API.
Done.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 102
> > <https://reviews.apache.org/r/17562/diff/2/?file=456989#file456989line102>
> >
> > This binding annotation belongs to the app, not the the http package. Please rename.
I am not sure what you mean here. Will ping you tomorrow for clarification.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 906-908
> > <https://reviews.apache.org/r/17562/diff/2/?file=456989#file456989line906>
> >
> > No need to create a new map every time - this can be an instance field set in the constructor.
Replaced this with GetInfoResult. Not moving it into constructor since constructing a response in the constructor seems weird and since the performance gains are negligible.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line 19
> > <https://reviews.apache.org/r/17562/diff/2/?file=456992#file456992line19>
> >
> > !==
Done
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line 21
> > <https://reviews.apache.org/r/17562/diff/2/?file=456992#file456992line21>
> >
> > !==
Thing we miss, when switching between languages. Done.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 31
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line31>
> >
> > This comment is not informative.
Removed.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 32
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line32>
> >
> > Rather than a "magic name" constant here you can just make clusterName a field in GetInfoResult to preserve type-safety and make generated code obvious.
>
> Suman Karumuri wrote:
> This is intentional.
Done.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, lines 34-36
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line34>
> >
> > Revert.
Removed magic name.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 412
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line412>
> >
> > Since the cluster name constant is defined here why not explicitly name it? In general I prefer the map<string, string> generalization only when the contents of the struct should somehow be "opaque" to the scheduler (e.g. defined by the client or defined by a plugin).
>
> Suman Karumuri wrote:
> Yes, it is intentionally kept generic.
Discussed in ticket.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, lines 454-457
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line454>
> >
> > Not clearly documented but this is a "special" API that shouldn't be deprecated - old clients use it to determine they are out of date and short-circuit.
>
> Suman Karumuri wrote:
> We deprecated the API to ensure that we don't program against it anymore. The plan is to delete it when all the old clients are gone. I plan to track it using the API metrics. Once the usage of the API goes to zero will remove it then.
>
> Also, I have filed AURORA-142 to move the aurora client over to the new API. AURORA-143 tracks, deleting the API task.
Discussed in ticket.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 459
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line459>
> >
> > "metadata" - no hyphen.
Done.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 460
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line460>
> >
> > How about getClusterInfo
>
> Suman Karumuri wrote:
> This is intentional.
Leaving this as getInfo to allow for future additions.
- Suman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33266
-----------------------------------------------------------
On Jan. 30, 2014, 10:27 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:27 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> >
Spoke to Bill before making these changes. We decided to use getInfo API and deprecate getAPIVersion. The getInfo will return with the clusterName and APIVersion for now. In future it may include other attributes and hence the flexible map.
Thanks for the code comments. Will incorporate them once the API changes are confirmed.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 412
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line412>
> >
> > Since the cluster name constant is defined here why not explicitly name it? In general I prefer the map<string, string> generalization only when the contents of the struct should somehow be "opaque" to the scheduler (e.g. defined by the client or defined by a plugin).
Yes, it is intentionally kept generic.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line 8
> > <https://reviews.apache.org/r/17562/diff/2/?file=456992#file456992line8>
> >
> > Any reason this isn't an instance field?
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 5
> > <https://reviews.apache.org/r/17562/diff/2/?file=456991#file456991line5>
> >
> > Is this necessary? Thought you could call chain these.
Yes. Call chaining makes for poorly readable and formatted code, so this is recommended. Further doing it this way, makes refactoring easier and cleaner.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, lines 454-457
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line454>
> >
> > Not clearly documented but this is a "special" API that shouldn't be deprecated - old clients use it to determine they are out of date and short-circuit.
We deprecated the API to ensure that we don't program against it anymore. The plan is to delete it when all the old clients are gone. I plan to track it using the API metrics. Once the usage of the API goes to zero will remove it then.
Also, I have filed AURORA-142 to move the aurora client over to the new API. AURORA-143 tracks, deleting the API task.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 460
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line460>
> >
> > How about getClusterInfo
This is intentional.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 32
> > <https://reviews.apache.org/r/17562/diff/2/?file=456993#file456993line32>
> >
> > Rather than a "magic name" constant here you can just make clusterName a field in GetInfoResult to preserve type-safety and make generated code obvious.
This is intentional.
- Suman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33266
-----------------------------------------------------------
On Jan. 30, 2014, 10:27 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:27 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 31, 2014, 12:56 a.m., Kevin Sweeney wrote:
> >
>
> Suman Karumuri wrote:
> Spoke to Bill before making these changes. We decided to use getInfo API and deprecate getAPIVersion. The getInfo will return with the clusterName and APIVersion for now. In future it may include other attributes and hence the flexible map.
>
> Thanks for the code comments. Will incorporate them once the API changes are confirmed.
>
> Kevin Sweeney wrote:
> For this case the magic name still doesn't make sense to me as it's defined in api.thrift. Why not just make it a field and maintain type-safety?
I agree with Kevin, for all intents and purposes you can consider the thrift struct a map. Formalizing this with field names gives a more consumable API.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33266
-----------------------------------------------------------
On Jan. 30, 2014, 10:27 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:27 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Kevin Sweeney <ke...@apache.org>.
> On Jan. 30, 2014, 4:56 p.m., Kevin Sweeney wrote:
> >
>
> Suman Karumuri wrote:
> Spoke to Bill before making these changes. We decided to use getInfo API and deprecate getAPIVersion. The getInfo will return with the clusterName and APIVersion for now. In future it may include other attributes and hence the flexible map.
>
> Thanks for the code comments. Will incorporate them once the API changes are confirmed.
For this case the magic name still doesn't make sense to me as it's defined in api.thrift. Why not just make it a field and maintain type-safety?
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33266
-----------------------------------------------------------
On Jan. 30, 2014, 2:27 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 2:27 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33266
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/17562/#comment62681>
This binding annotation belongs to the app, not the the http package. Please rename.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/17562/#comment62662>
No need to create a new map every time - this can be an instance field set in the constructor.
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
<https://reviews.apache.org/r/17562/#comment62664>
Is this necessary? Thought you could call chain these.
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js
<https://reviews.apache.org/r/17562/#comment62682>
Any reason this isn't an instance field?
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js
<https://reviews.apache.org/r/17562/#comment62666>
!==
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js
<https://reviews.apache.org/r/17562/#comment62667>
!==
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62675>
This comment is not informative.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62674>
Rather than a "magic name" constant here you can just make clusterName a field in GetInfoResult to preserve type-safety and make generated code obvious.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62676>
Revert.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62680>
Since the cluster name constant is defined here why not explicitly name it? In general I prefer the map<string, string> generalization only when the contents of the struct should somehow be "opaque" to the scheduler (e.g. defined by the client or defined by a plugin).
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62677>
Not clearly documented but this is a "special" API that shouldn't be deprecated - old clients use it to determine they are out of date and short-circuit.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62679>
"metadata" - no hyphen.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment62678>
How about getClusterInfo
- Kevin Sweeney
On Jan. 30, 2014, 2:27 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 2:27 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
> On Feb. 4, 2014, 8:52 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 231
> > <https://reviews.apache.org/r/17562/diff/3/?file=464021#file464021line231>
> >
> > Please revert. This is an anti-pattern, as tests can no longer alter the value. Ideally, the value would be plumbed through a module, but the overloaded constructor is a compromise.
done.
> On Feb. 4, 2014, 8:52 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 184
> > <https://reviews.apache.org/r/17562/diff/3/?file=464021#file464021line184>
> >
> > How about a binding for GetInfoResult instead? This will leave us in a better position to get rid of @ClusterName in the near future, which would be nice.
Did this in APIVersionInterceptor.
> On Feb. 4, 2014, 8:52 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 456
> > <https://reviews.apache.org/r/17562/diff/3/?file=464025#file464025line456>
> >
> > Thinking about this more, we could save the extra round-trip if the server info is embedded in every Response. APIVersionInterceptor already does something like this, embedding the version number in all responses. What do you think about that?
Done.
- Suman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33635
-----------------------------------------------------------
On Feb. 20, 2014, 9:53 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:53 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java ec56c649116c03ef148bac916bd6691a94685bc3
> src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 12113532eb062308af3c43458661c7b43b23237f
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 43a24969887b891339c97d4a0a055488c9188a5d
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 656e44eb51508c7ab05af8b91e9dcec216d48984
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 10f5cdab16230e66ac7c9fab199e2dd2564248da
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review33635
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/17562/#comment63183>
How about a binding for GetInfoResult instead? This will leave us in a better position to get rid of @ClusterName in the near future, which would be nice.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/17562/#comment63184>
Please revert. This is an anti-pattern, as tests can no longer alter the value. Ideally, the value would be plumbed through a module, but the overloaded constructor is a compromise.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment63185>
Thinking about this more, we could save the extra round-trip if the server info is embedded in every Response. APIVersionInterceptor already does something like this, embedding the version number in all responses. What do you think about that?
- Bill Farner
On Feb. 4, 2014, 8:15 a.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 8:15 a.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review35360
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java
<https://reviews.apache.org/r/17562/#comment65833>
resp.setServerInfo
- Kevin Sweeney
On Feb. 24, 2014, 4:56 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 4:56 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
> src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
> src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
> src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
> src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
> src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
> src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
> On Feb. 25, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java, line 195
> > <https://reviews.apache.org/r/17562/diff/6/?file=503050#file503050line195>
> >
> > doc here has drifted, please being it back in check
Done.
> On Feb. 25, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 413
> > <https://reviews.apache.org/r/17562/diff/6/?file=503059#file503059line413>
> >
> > A brief doc would go a long way here. What's this used for? Where do the fields go?
Done.
> On Feb. 25, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 439
> > <https://reviews.apache.org/r/17562/diff/6/?file=503059#file503059line439>
> >
> > Convention here is to pseudo-mangle the name by tacking DEPRECATED on the name. This has zero impact on the wire, and serves in place of compiler-assisted deprecation warnings.
Good idea. Done.
> On Feb. 25, 2014, 8:10 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, line 176
> > <https://reviews.apache.org/r/17562/diff/6/?file=503060#file503060line176>
> >
> > Mind putting some dummy data in ServerInfo fields?
Done.
- Suman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review35447
-----------------------------------------------------------
On Feb. 25, 2014, 1:22 a.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 1:22 a.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
> src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
> src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
> src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
> src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
> src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
> src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review35447
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java
<https://reviews.apache.org/r/17562/#comment65966>
doc here has drifted, please being it back in check
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment65969>
A brief doc would go a long way here. What's this used for? Where do the fields go?
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment65970>
Convention here is to pseudo-mangle the name by tacking DEPRECATED on the name. This has zero impact on the wire, and serves in place of compiler-assisted deprecation warnings.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment65971>
No action necessary on your part, but the TODO is the best you can do here. Thrift routes messages to handler methods based on the RPC method _name_, so name mangling here would actually break wire compatibility.
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
<https://reviews.apache.org/r/17562/#comment65972>
Mind putting some dummy data in ServerInfo fields?
- Bill Farner
On Feb. 25, 2014, 1:22 a.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 1:22 a.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
> src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
> src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
> src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
> src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
> src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
> src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated March 6, 2014, 11:43 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Fixed code review feedback.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
build.gradle ba83b913808ed41663e72de25f5751290057535d
src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
src/main/thrift/org/apache/aurora/gen/api.thrift 2afbb4ce98e49332495d264218a7112bce8d450b
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java cd532bdd2d34b4636d0c22494512d53a2f089463
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java f5effe741fc37155803258fb0a3a6c7f33c2a65e
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
src/test/resources/org/apache/aurora/gen/api.thrift.md5 5cfa330f54c0297c84b8f41ee550a7bc6f163010
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated Feb. 25, 2014, 1:22 a.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Fixed issue.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated Feb. 25, 2014, 12:56 a.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Now injecting IServerInfo.
Updated ServerInfoInterceptor to always override version and serverInfo. Updated associated tests.
Dropped ClusterName interface. Now using the injected IServerInfo instead.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java, line 109
> > <https://reviews.apache.org/r/17562/diff/4/?file=499447#file499447line109>
> >
> > Rather than piecemeal equality checks, check the whole message. This is more future-proof as fields are added:
> >
> > assertEquals(expectedServerInfo, decoratedRsponse.serverInfo);
Done
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java, line 118
> > <https://reviews.apache.org/r/17562/diff/4/?file=499447#file499447line118>
> >
> > The line break style here is rather arbitrary. Consider this instead:
> >
> > Response response = new Response()
> > .setResponseCode(OK)
> > .setResult(Result.getJobsResult(
> > new GetJobsResult()
> > .setConfigs(ImmutableSet.<JobConfiguration>of())))
> > .setServerInfo(
> > new ServerInfo()
> > .setClusterName(CLUSTER_NAME)
> > .setThriftAPIVersion(thriftVersion));
> >
> > Here, the wrap level directly reflects the nesting depth, and future addition of chained or nested method calls does not trigger a ripple.
Done.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java, line 100
> > <https://reviews.apache.org/r/17562/diff/4/?file=499447#file499447line100>
> >
> > This helper looks like it would get some mileage in here:
> >
> > private static Response okResponse(Result result) {
> > return new Response()
> > .setResponseCode(OK)
> > .setResult(result);
> > }
Yeah, it is useful. Using it now.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 415
> > <https://reviews.apache.org/r/17562/diff/4/?file=499444#file499444line415>
> >
> > This was previously intentionally i32, why the change?
Reverting to i32 after offline discussion.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java, line 28
> > <https://reviews.apache.org/r/17562/diff/4/?file=499440#file499440line28>
> >
> > Please inject this.
Done.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java, line 41
> > <https://reviews.apache.org/r/17562/diff/4/?file=499440#file499440line41>
> >
> > I'm not sure why we did the null check in the first place, but i can't think of a good reason for upstream code to be setting this to anything other than these values. I say blindly overwrite anything already set for both.
Done.
> On Feb. 22, 2014, 6:35 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 171
> > <https://reviews.apache.org/r/17562/diff/4/?file=499438#file499438line171>
> >
> > The cluster name string is bound, annotated with @ClusterName, so you can avoid the plumbing and just:
> >
> > @Inject
> > @ClusterName
> > private String clusterName;
> >
> > In the target classes.
Done.
- Suman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review35230
-----------------------------------------------------------
On Feb. 25, 2014, 12:56 a.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 12:56 a.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> build.gradle f8f00e6e73e566795d8208ca7edd6e70fc9c5d76
> src/main/java/org/apache/aurora/scheduler/app/AppModule.java 296bc1f7f20c4d8bb0ce14dfd4f9c003473ceec7
> src/main/java/org/apache/aurora/scheduler/http/ClusterName.java ac77f6ac0b71d5b3735cb269c9bfff3e2c6fda20
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 3686a6682d39fa75baddfb4a1b739e14e2caf508
> src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java 25ba7da5f8bbe5416f41bb0b14850beb84392cc7
> src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 36aa425e78c519188827c3455a0aef53b060d8eb
> src/main/java/org/apache/aurora/scheduler/http/Slaves.java cc8971ca6f4a402064a843eef5a23a47d2df06c1
> src/main/java/org/apache/aurora/scheduler/http/Utilization.java 0cbdeeafde9db05e71fdb7e71b6d4529dee9cbdd
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 94189642a4f35aa0cad8176589cfbd84964c2e14
> src/main/thrift/org/apache/aurora/gen/api.thrift cd60f47bf34b4a634004e2ad9eadad37aa1556bb
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java f0f0e39c40758cd40c981bcbde1e59f2e7a99e3f
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 fafb5100443482e662db453429c5259f2ab80ae5
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/#review35230
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
<https://reviews.apache.org/r/17562/#comment65681>
The cluster name string is bound, annotated with @ClusterName, so you can avoid the plumbing and just:
@Inject
@ClusterName
private String clusterName;
In the target classes.
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java
<https://reviews.apache.org/r/17562/#comment65682>
Please inject this.
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java
<https://reviews.apache.org/r/17562/#comment65683>
I'm not sure why we did the null check in the first place, but i can't think of a good reason for upstream code to be setting this to anything other than these values. I say blindly overwrite anything already set for both.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/17562/#comment65689>
This was previously intentionally i32, why the change?
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java
<https://reviews.apache.org/r/17562/#comment65688>
This helper looks like it would get some mileage in here:
private static Response okResponse(Result result) {
return new Response()
.setResponseCode(OK)
.setResult(result);
}
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java
<https://reviews.apache.org/r/17562/#comment65684>
s/final //
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java
<https://reviews.apache.org/r/17562/#comment65685>
This line is redundant with the line below.
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java
<https://reviews.apache.org/r/17562/#comment65686>
Rather than piecemeal equality checks, check the whole message. This is more future-proof as fields are added:
assertEquals(expectedServerInfo, decoratedRsponse.serverInfo);
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java
<https://reviews.apache.org/r/17562/#comment65687>
The line break style here is rather arbitrary. Consider this instead:
Response response = new Response()
.setResponseCode(OK)
.setResult(Result.getJobsResult(
new GetJobsResult()
.setConfigs(ImmutableSet.<JobConfiguration>of())))
.setServerInfo(
new ServerInfo()
.setClusterName(CLUSTER_NAME)
.setThriftAPIVersion(thriftVersion));
Here, the wrap level directly reflects the nesting depth, and future addition of chained or nested method calls does not trigger a ripple.
- Bill Farner
On Feb. 20, 2014, 9:53 p.m., Suman Karumuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17562/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:53 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-120
> https://issues.apache.org/jira/browse/AURORA-120
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Added getInfo API that returns page title and thrift API version.
>
> Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
>
> Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java ec56c649116c03ef148bac916bd6691a94685bc3
> src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 12113532eb062308af3c43458661c7b43b23237f
> src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
> src/main/thrift/org/apache/aurora/gen/api.thrift 43a24969887b891339c97d4a0a055488c9188a5d
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 656e44eb51508c7ab05af8b91e9dcec216d48984
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
> src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 10f5cdab16230e66ac7c9fab199e2dd2564248da
>
> Diff: https://reviews.apache.org/r/17562/diff/
>
>
> Testing
> -------
>
> gradle clean build.
> gradle run.
>
>
> Thanks,
>
> Suman Karumuri
>
>
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 9:53 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Moved serverInfo into APIVersionInterceptor.
Renamed APIVersionInterceptor to ServerInfoInterceptor.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java ec56c649116c03ef148bac916bd6691a94685bc3
src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 12113532eb062308af3c43458661c7b43b23237f
src/main/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptor.java e3e050e493e4a69e1d424d3382cb37f7ce6b7a4c
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java c68977a7cef99a31baca2043618df304298b407e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1916ff9c02216da906af4e6164212faeff37995e
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
src/main/thrift/org/apache/aurora/gen/api.thrift 43a24969887b891339c97d4a0a055488c9188a5d
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 656e44eb51508c7ab05af8b91e9dcec216d48984
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2254574a91c5a7bf2ddc7d8a7d31c3e4e360486d
src/test/java/org/apache/aurora/scheduler/thrift/aop/APIVersionInterceptorTest.java 2f0b6716fe2999c34c0351f03b6b4c1532eafee0
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d93ac53ec15863f2ee0e0ebc4737d8f4f8e4be9d
src/test/resources/org/apache/aurora/gen/api.thrift.md5 10f5cdab16230e66ac7c9fab199e2dd2564248da
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated Feb. 4, 2014, 8:15 a.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Addressed code review feedback.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri
Re: Review Request 17562: Added cluster name to page title.
Posted by Suman Karumuri <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17562/
-----------------------------------------------------------
(Updated Jan. 30, 2014, 10:27 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Bugs: AURORA-120
https://issues.apache.org/jira/browse/AURORA-120
Repository: aurora
Description
-------
Added getInfo API that returns page title and thrift API version.
Added a TitleController to UI to set the page title. The controller queries the getInfo API to get the clusterName which is now included in the page title.
Deprecated getVersion API in favor of getInfo to obtain the thrift API version.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 822b8b9e3348677ff2f45da2f76ae9a7a96a41b6
src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 2601da9ef64ef91cbf9db9b6f12a8d2cdbb0c297
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0abd3e737e901f08d18c3ceb55ea2f94847cc2e1
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js a0385e95c8c1fc950f507511377dcebe35f258a7
src/main/thrift/org/apache/aurora/gen/api.thrift 94569f9abfb56c50e67e09cc018484463f9de427
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 02b7a27e0a9ec5226d9d043de8bf4739fb151b09
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 644d6e87f5ed44d2705ebbcc6619a727d52c03d8
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 62fc8045f6a5fda234df73452685bd04e3142aaf
src/test/resources/org/apache/aurora/gen/api.thrift.md5 e88d29732ee8652ede6c7ee13513122086385646
Diff: https://reviews.apache.org/r/17562/diff/
Testing
-------
gradle clean build.
gradle run.
Thanks,
Suman Karumuri