You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2014/10/16 23:03:36 UTC
Review Request 26823: Make paging parameters available to individual
resource handlers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/
-----------------------------------------------------------
Review request for Ambari, Nate Cole and Tom Beerbower.
Bugs: AMBARI-7817
https://issues.apache.org/jira/browse/AMBARI-7817
Repository: ambari
Description
-------
Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
Implemented the new framework for alert history and notices.
Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
Diff: https://reviews.apache.org/r/26823/diff/
Testing
-------
New tests written to cover alerts and ResourceProviderPageResponse.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:00 min
[INFO] Finished at: 2014-10-16T17:03:28-04:00
[INFO] Final Memory: 29M/239M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57180
-----------------------------------------------------------
Ship it!
Ship It!
- Tom Beerbower
On Oct. 17, 2014, 5:52 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 5:52 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java da6c886
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PageRequestImpl.java 5f2bd8a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java c02fdd4
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java 66a04b9
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/
-----------------------------------------------------------
(Updated Oct. 17, 2014, 1:52 p.m.)
Review request for Ambari, Nate Cole and Tom Beerbower.
Changes
-------
Page and sort request/response information is now separated out on the request.
Bugs: AMBARI-7817
https://issues.apache.org/jira/browse/AMBARI-7817
Repository: ambari
Description
-------
Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
Implemented the new framework for alert history and notices.
Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java da6c886
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PageRequestImpl.java 5f2bd8a
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java c02fdd4
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java 66a04b9
ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
Diff: https://reviews.apache.org/r/26823/diff/
Testing
-------
New tests written to cover alerts and ResourceProviderPageResponse.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:00 min
[INFO] Finished at: 2014-10-16T17:03:28-04:00
[INFO] Final Memory: 29M/239M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/
-----------------------------------------------------------
(Updated Oct. 17, 2014, 12:34 p.m.)
Review request for Ambari, Nate Cole and Tom Beerbower.
Changes
-------
I addressed most of the points that Tom pointed out except for where the ResourceProviderPageResponse should be. I tried it on the PageRequest, but it was weird in the cases where I just requested a sort; in those cases, I had to create my own PageRequestImpl with dummy information in order to prevent the ClusterControllerImpl from re-sorting the data.
I'm open to more ideas if we don't like where ResourceProviderPageResponse is.
Bugs: AMBARI-7817
https://issues.apache.org/jira/browse/AMBARI-7817
Repository: ambari
Description
-------
Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
Implemented the new framework for alert history and notices.
Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java da6c886
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PageRequestImpl.java 5f2bd8a
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java c02fdd4
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java 66a04b9
ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
Diff: https://reviews.apache.org/r/26823/diff/
Testing
-------
New tests written to cover alerts and ResourceProviderPageResponse.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:00 min
[INFO] Finished at: 2014-10-16T17:03:28-04:00
[INFO] Final Memory: 29M/239M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57138
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Oct. 16, 2014, 5:03 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 5:03 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java, lines 105-129
> > <https://reviews.apache.org/r/26823/diff/1/?file=723311#file723311line105>
> >
> > It's kind of strange to have a response in the request, I think. I guess there's not a great way to do it since getResources doesn't allow you to pass anything back but the set of resources. Would it be cleaner to bundle this up along with the PageRequest? That way at least all the page related info is kept together.
>
> Jonathan Hurley wrote:
> What about the case where a PageRequest is null but there is a SortRequest? In that case, we would not be able to indicate that the ResourceProvider did the sorting on its own and the ClusterController would try to sort it again, potentially changing a correct ordering. That's why I put it on the Request. Thoughts?
>
> Tom Beerbower wrote:
> Sort of thinking out loud here ...
>
> PageInfo {
> int totalCount;
> boolean pagedResponse;
> PageRequest request;
> }
>
> SortInfo {
> boolean sortedResponse;
> SortRequest;
> }
>
> Then we can construct the Request and optionaly pass in PageInfo and/or SortInfo. When the RP returns we can check those objects to see if the results are paged and/or sorted.
>
> I realize that it's basically what you have already in a slightly different form. I would just prefer to keep all the page and sort stuff together... up to you.
OK, I see where you're heading. Let me take a crack at this and we'll see if we like how it turns out.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57140
-----------------------------------------------------------
On Oct. 17, 2014, 12:34 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 12:34 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java da6c886
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PageRequestImpl.java 5f2bd8a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java c02fdd4
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java 66a04b9
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java, lines 80-103
> > <https://reviews.apache.org/r/26823/diff/1/?file=723311#file723311line80>
> >
> > I'd prefer to not have the setters in the interface, if possible.
I can remove the setters by moving them into the RequestImpl and using PropertyHelper to set them.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java, lines 466-472
> > <https://reviews.apache.org/r/26823/diff/1/?file=723304#file723304line466>
> >
> > Could we move this logic into PropertyHelper.getReadRequest() passing in the sort and page info? That way we can get an immutable Request object through createRequest() and we don't have to expose the setters on the Request interface.
Yes, I think I can move these in there.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java, lines 211-213
> > <https://reviews.apache.org/r/26823/diff/1/?file=723309#file723309line211>
> >
> > So the resource provider needs to be able to determine whether or not it can return a paged response. I guess that it's safe for the RP to do the paging as long as it knows about all of the properties in the predicate. Is that what you are thinking?
Yes, that's what I was thinking. It should only put the ResourceProviderPageResponse on the request if it was able to handle it itself.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java, line 25
> > <https://reviews.apache.org/r/26823/diff/1/?file=723312#file723312line25>
> >
> > typo **presence**
Fixed.
> On Oct. 17, 2014, 11:25 a.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java, lines 105-129
> > <https://reviews.apache.org/r/26823/diff/1/?file=723311#file723311line105>
> >
> > It's kind of strange to have a response in the request, I think. I guess there's not a great way to do it since getResources doesn't allow you to pass anything back but the set of resources. Would it be cleaner to bundle this up along with the PageRequest? That way at least all the page related info is kept together.
What about the case where a PageRequest is null but there is a SortRequest? In that case, we would not be able to indicate that the ResourceProvider did the sorting on its own and the ClusterController would try to sort it again, potentially changing a correct ordering. That's why I put it on the Request. Thoughts?
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57140
-----------------------------------------------------------
On Oct. 16, 2014, 5:03 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 5:03 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Oct. 17, 2014, 3:25 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java, lines 105-129
> > <https://reviews.apache.org/r/26823/diff/1/?file=723311#file723311line105>
> >
> > It's kind of strange to have a response in the request, I think. I guess there's not a great way to do it since getResources doesn't allow you to pass anything back but the set of resources. Would it be cleaner to bundle this up along with the PageRequest? That way at least all the page related info is kept together.
>
> Jonathan Hurley wrote:
> What about the case where a PageRequest is null but there is a SortRequest? In that case, we would not be able to indicate that the ResourceProvider did the sorting on its own and the ClusterController would try to sort it again, potentially changing a correct ordering. That's why I put it on the Request. Thoughts?
Sort of thinking out loud here ...
PageInfo {
int totalCount;
boolean pagedResponse;
PageRequest request;
}
SortInfo {
boolean sortedResponse;
SortRequest;
}
Then we can construct the Request and optionaly pass in PageInfo and/or SortInfo. When the RP returns we can check those objects to see if the results are paged and/or sorted.
I realize that it's basically what you have already in a slightly different form. I would just prefer to keep all the page and sort stuff together... up to you.
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57140
-----------------------------------------------------------
On Oct. 17, 2014, 4:34 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 4:34 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java da6c886
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PageRequestImpl.java 5f2bd8a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java c02fdd4
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java 66a04b9
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 26823: Make paging parameters available to
individual resource handlers
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26823/#review57140
-----------------------------------------------------------
Looks good. A few comments.
ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java
<https://reviews.apache.org/r/26823/#comment97602>
Could we move this logic into PropertyHelper.getReadRequest() passing in the sort and page info? That way we can get an immutable Request object through createRequest() and we don't have to expose the setters on the Request interface.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java
<https://reviews.apache.org/r/26823/#comment97603>
So the resource provider needs to be able to determine whether or not it can return a paged response. I guess that it's safe for the RP to do the paging as long as it knows about all of the properties in the predicate. Is that what you are thinking?
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java
<https://reviews.apache.org/r/26823/#comment97604>
haha ... you realy dont like 'this.'
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java
<https://reviews.apache.org/r/26823/#comment97606>
I'd prefer to not have the setters in the interface, if possible.
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java
<https://reviews.apache.org/r/26823/#comment97608>
It's kind of strange to have a response in the request, I think. I guess there's not a great way to do it since getResources doesn't allow you to pass anything back but the set of resources. Would it be cleaner to bundle this up along with the PageRequest? That way at least all the page related info is kept together.
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java
<https://reviews.apache.org/r/26823/#comment97611>
typo **presence**
- Tom Beerbower
On Oct. 16, 2014, 9:03 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26823/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:03 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-7817
> https://issues.apache.org/jira/browse/AMBARI-7817
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Exposed pagination and sorting via the SPI Request so that concrete ResourceProviders can do their own paging and sorting. The providers are responsibile for setting the returned ResourceProviderPageResponse so that the ClusterController knows not to perform in-memory processing of the result set.
>
> Implemented the new framework for alert history and notices.
>
> Currently, the Alert DAOs return 0 for the total count of items since I don't yet know if the UI will need this information. If we determine that we want to implement it, there are stub methods in the DAOs for getting the counts. I didn't want to incur the JPA penalty if it wasn't needed.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaPredicateVisitor.java 660bae7
> ambari-server/src/main/java/org/apache/ambari/server/api/query/JpaSortBuilder.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 6dfdb49
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertHistoryRequest.java 9dc6cc4
> ambari-server/src/main/java/org/apache/ambari/server/controller/AlertNoticeRequest.java 40c7c67
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 300f02f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java c4a96ff
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 5d1024a
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java 93eaf0a
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java 13a2856
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ResourceProviderPageResponse.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java a49a04a
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 8414765
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 261c95d
> ambari-server/src/test/java/org/apache/ambari/server/orm/AlertDaoHelper.java e5a5638
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDispatchDAOTest.java 430bede
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java 5886ab4
>
> Diff: https://reviews.apache.org/r/26823/diff/
>
>
> Testing
> -------
>
> New tests written to cover alerts and ResourceProviderPageResponse.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:00 min
> [INFO] Finished at: 2014-10-16T17:03:28-04:00
> [INFO] Final Memory: 29M/239M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>