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



ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java
<https://reviews.apache.org/r/30120/#comment113445>

    it is ok to pass in a null value for ignoredProperties so you can always 
use the same form of the method



ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java
<https://reviews.apache.org/r/30120/#comment113446>

    This cast isn't safe, a resource definition is free to return any 
collection from the get*Directives methods.  
    
    Sorry about using a Set in the compiler, there is no need for it to be a 
set and should have been a collection.  
    
    You can either replace the cast with new HashSet(ignoredProperties) or 
change the compiler/lexer to take a collection and remove the need to cast.



ambari-server/src/main/java/org/apache/ambari/server/api/services/RequestFactory.java
<https://reviews.apache.org/r/30120/#comment113449>

    batchCreate = !applyEligibleDirectives(Request.Type.POST, body, uriInfo, 
resource);



ambari-server/src/main/java/org/apache/ambari/server/api/services/RequestFactory.java
<https://reviews.apache.org/r/30120/#comment113451>

    Confusing name for this method.  'applyDirectives' or 
'parseRequestDirectives' would be better in my opinion.  Not all query params 
are directives, some are actual query properties.  Using 
allDirecitivesApplicable implies that the requrest contains illegal directives 
when really there is not such thing as a non-applicable directive.  Either the 
query param is a directive or it is something else.  The return value is 
indicating if query params were specified, not whether an invalid directive was 
specified.


- John Speidel


On Jan. 21, 2015, 2:28 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30120/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 2:28 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Nettleton, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9230
>     https://issues.apache.org/jira/browse/AMBARI-9230
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add `getUpdateDirectives` method to 
> `org.apache.ambari.server.api.resources.ResourceDefinition`:
> 
> ```
>   public Collection<String> getUpdateDirectives();
> ```
> 
> Add default implementation to 
> `org.apache.ambari.server.api.resources.BaseResourceDefinition` to return an 
> empty Set
> 
> Update `org.apache.ambari.server.api.services.RequestFactory` to process 
> _update directives_ like it processes _create directives_ - see 
> `org.apache.ambari.server.api.services.RequestFactory#createPostRequest`
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java
>  f98779c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceDefinition.java
>  7632e64 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java
>  7494491 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/RequestFactory.java
>  649e210 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/BaseRequestTest.java
>  27fc077 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/RequestFactoryTest.java
>  5c56670 
> 
> Diff: https://reviews.apache.org/r/30120/diff/
> 
> 
> Testing
> -------
> 
> #Jenkins test results:
> 
> Running org.apache.ambari.server.api.services.RequestFactoryTest
> Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.351 sec
> 
> Running org.apache.ambari.server.api.services.GetRequestTest
> Tests run: 16, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.959 sec
> 
> Running org.apache.ambari.server.api.services.PostRequestTest
> Tests run: 16, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.911 sec
> 
> Running org.apache.ambari.server.api.services.PutRequestTest
> Tests run: 16, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.902 sec
> 
> Running org.apache.ambari.server.api.services.DeleteRequestTest
> Tests run: 16, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.902 sec
> 
> Complete ambari-server tests
> Tests run: 2576, Failures: 0, Errors: 0, Skipped: 15
> 
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 01:11 h
> [INFO] Finished at: 2015-01-21T13:43:07+00:00
> [INFO] Final Memory: 43M/514M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Robert Levas
> 
>

Reply via email to