----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28336/#review62584 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java <https://reviews.apache.org/r/28336/#comment104663> This seems to be confusing since the caller sees X number of `Resources.Type` arguments that do not indicate any hierarchy, yet the implementation takes the first as the main type and the rest as the sub types. This type of syntax seems to make better sense when all of the arguements are used the same way. I would rather see something like `public BaseResourceDefinition(Resource.Type resourceType, Collection<Resource.Type> subTypes)` making the invocation of this constructor look like: `super(Resource.Type.Request, Arrays.asList(Resource.Type.State, Resource.Type.Task))` Which looks clearer (to me) than: `super(Resource.Type.Request, Resource.Type.State, Resource.Type.Task)` If there are no other issues and no on else cares about this, then don't let this hold up progress. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java <https://reviews.apache.org/r/28336/#comment104670> Is there any particular reason this is an _inner_ class. Technically is it not really an inner class. That said, I am typically a fan of the `private static (inner) class`; however this is a `protected static (inner) class` in an _implementation_ class (not a class one would expect to be used as a base class). Might it be better to move this out to its own class? Maybe it will be useful elsewhere? ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java <https://reviews.apache.org/r/28336/#comment104671> The comment here is confusing since we are counting non-`COMPLETED` statuses twice. We just don't want to doubly count the `COMPLETED` statuses twice. It took a little digging to figure out that this is not a bug due to how the counts are used elsewhere in this class. - Robert Levas On Nov. 21, 2014, 12:26 p.m., Tom Beerbower wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28336/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2014, 12:26 p.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/StageResourceDefinition.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/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. > > > Thanks, > > Tom Beerbower > >
