ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Speidel" <jspei...@hortonworks.com>
Subject Re: Review Request 27700: Allow for server-side commands
Date Tue, 11 Nov 2014 19:27:53 GMT

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


Overall, looks good.


ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java
<https://reviews.apache.org/r/27700/#comment102207>

    Use braces for all if statements.  Not sure if this is explicitly called out in our coding
standards but not using braces is generally frowned upon as it is error prone.



ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
<https://reviews.apache.org/r/27700/#comment102210>

    javadoc
    
    Seems odd that only the setters are in the interface



ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
<https://reviews.apache.org/r/27700/#comment102211>

    javadoc



ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
<https://reviews.apache.org/r/27700/#comment102212>

    typo
    is -> it's



ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
<https://reviews.apache.org/r/27700/#comment102213>

    javadoc for members



ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
<https://reviews.apache.org/r/27700/#comment102232>

    Should consider using an executor instead of directly creating threads here.  This will
simplify and provide more flexibility if we choose to change the concurrency strategy for
server tasks.  Generally using an executor is preferred to rolling your own unless you have
a really good reason to do otherwise.



ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
<https://reviews.apache.org/r/27700/#comment102228>

    if join() is interrupted, do we want to propagate the interrupt to the worker thread?



ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java
<https://reviews.apache.org/r/27700/#comment102233>

    class is missing all javadoc's


- John Speidel


On Nov. 10, 2014, 2:05 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27700/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 2:05 p.m.)
> 
> 
> Review request for Ambari, dilli dorai, Jonathan Hurley, John Speidel, Nate Cole, Robert
Nettleton, and Sid Wagle.
> 
> 
> Bugs: AMBARI-7985
>     https://issues.apache.org/jira/browse/AMBARI-7985
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Ambari currently handles client-/agent-side commands; however there is no ability to
handle server-side commands. Server-side commands should be specified as a task in a stage
and managed along with the stage.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessor.java
1f99b4a 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
5e879cc 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionManager.java
e2fad5f 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java
81fee75 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java bbc5ac3

>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java
52b0ba6 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
6920a9e 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerAction.java
be885b5 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManager.java
011cf06 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManagerImpl.java
3a16c77 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEvent.java
78590fc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEventType.java
b43ac9c 
>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostUpgradeEvent.java
8b375fe 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
36acbc2 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
5a2c467 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
7224924 
>   ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
6e78b1d 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
b2c023f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
PRE-CREATION 
>   ambari-server/src/test/python/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27700/diff/
> 
> 
> Testing
> -------
> 
> -------------------------------------------------------------------------------
> Test set: org.apache.ambari.server.serveraction.ServerActionExecutorTest
> -------------------------------------------------------------------------------
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.822 sec
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


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