> On Nov. 24, 2014, 8:31 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java,
> >  line 186
> > <https://reviews.apache.org/r/28336/diff/4/?file=773593#file773593line186>
> >
> >     How does this request inform the caller that pagination and sorting was 
> > used? It constructs the historyRequest, but I can't see where the response 
> > indicates that the resources are paged or sorted.
> 
> Tom Beerbower wrote:
>     The cluster controller now calls queryForResources instead of 
> getResources if the resource provider implements it.
>     
>     So, if a resource provider is going to do its own sorting and/or paging 
> it should implement ExtendedResourceProvider.

Thanks for the explanation. Nice!


> On Nov. 24, 2014, 8:31 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity_.java,
> >  lines 35-46
> > <https://reviews.apache.org/r/28336/diff/4/?file=773607#file773607line35>
> >
> >     Copy/paste error?
> 
> Tom Beerbower wrote:
>     Yikes! Good catch.  Obviously I used AlertHistoryEntity_ as the template. 
>  I'm a little surprised that the JPA mapping works with it like that, but it 
> looks like the type information isn't actually being used.  The mapping works 
> fine when I run the code.  Should getPredicateMapping include the entity type 
> info?  ...
>     
>           public static Map<String, List<? extends 
> SingularAttribute<StageEntity, ?>>> getPredicateMapping() {
>             Map<String, List<? extends SingularAttribute<StageEntity, ?>>> 
> mapping = new HashMap<String, List<? extends SingularAttribute<StageEntity, 
> ?>>>();
>     
>     The same would apply to AlertHistoryEntity_.

I would have expected a failure as well; odd that it doesn't. Yes, I'd say the 
type information would be good to have to catch other copy/paste errors. 
Actually, are you sure that the JPA code is being invoked correctly for the 
StageResourceProvider? I found that when I was writing the alert JPA stuff, 
there was post-processing happening by the cluster controller which would sort 
and paginate my results again (hence the methods that tell the controller not 
to do that). But associated with this was a bug that I had where my DAO was not 
properly using the sort/page info given to it by the resource provider, but it 
was being hidden by the double-processing.

I wrote a few methods in AlertsDAOTest which specifically exercise the JPA code 
directly (```testAlertHistoryPredicate()``` and 
```testAlertHistoryPagination()``` and ```testAlertHistorySorting()```). This 
way I knew I was getting a paged/sorted result set from JPA correctly before it 
even hit my resource provider. Should StageDAO have some of these tests too?


- Jonathan


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


On Nov. 24, 2014, 9:27 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28336/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2014, 9:27 a.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, John Speidel, Nate Cole, and 
> Robert Levas.
> 
> 
> Bugs: AMBARI-8163
>     https://issues.apache.org/jira/browse/AMBARI-8163
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently, it is possible to query Ambari (via the REST API) for details 
> about _asynchronous_ requests and their related tasks. This useful when 
> trying to obtain progress information.  However, some information necessary 
> for the UI to indicate meaningful progress is not available.  This 
> information is related to the stages that are generated. 
> 
> *NOTE:* Each _asynchronous_ request is broken down into 1 or more stages and 
> each stage contains 1 or more tasks.
> 
> If stage information was available via the REST API, it would be possible for 
> the caller (maybe a UI) to track high-level tasks (at the {{stage}} level) 
> rather than each lower-level unit of work (at the {{task}} level).   
> 
> To allow for this, a new API resource (and associated handler) needs to be 
> created.  The resource should be read-only (like {{requests}} and {{tasks}}), 
> and should provide information stored in the {{stage}} table from the Ambari 
> database.  
> 
> The following properties should be returned for each {{stage}}:
> 
> * stage_id
> * request_id
> * cluster_id
> * request_context 
> ** _This should probably be renamed to something more appropriate, like 
> stage_context, stage_name, or etc..._
> * start_time
> * end_time
> * progress_percent
> * status
> 
> It is expected that the resources would be queried using:
> 
> {code}
> GET  /api/v1/clusters/{clusterid}/requests/{requestid}/stages
> {code}
> 
> Also, some subset of the stage data should be provided when querying for 
> details about a specific {{request}}, like in:
> 
> {code}
> GET  /api/v1/clusters/{clusterid}/requests/{requestid}
> {code}
> 
> See {{request}} and {{task}} resource for examples.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java 
> a1c4882 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java
>  a5a7234 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/RequestResourceDefinition.java
>  291b01a 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
>  ba3f32f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/RequestService.java
>  fc1b515 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/StageService.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/TaskService.java
>  ade8d9c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java
>  409aace 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java
>  956f710 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java
>  ec40c4f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java
>  d0ce1cf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QueryResponseImpl.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestImpl.java
>  9b98737 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ClusterController.java
>  35ea680 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ExtendedResourceProvider.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/QueryResponse.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Request.java
>  92c5db5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java
>  b71b43c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java
>  3e2111e 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java 
> 900dbeb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity_.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java
>  cfb2efb 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QueryResponseImplTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StageResourceProviderTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28336/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> Results :
> 
> Tests run: 2288, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 26:48 min
> [INFO] Finished at: 2014-11-22T08:06:15-05:00
> [INFO] Final Memory: 41M/485M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Reply via email to