ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Hurley" <jhur...@hortonworks.com>
Subject Re: Review Request 26823: Make paging parameters available to individual resource handlers
Date Fri, 17 Oct 2014 16:53:13 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message