You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by John Speidel <js...@hortonworks.com> on 2015/05/13 05:41:07 UTC
Review Request 34157: Provide host predicate property validation for
blueprint requests
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34157/
-----------------------------------------------------------
Review request for Ambari, Robert Levas and Robert Nettleton.
Bugs: AMBARI-11093
https://issues.apache.org/jira/browse/AMBARI-11093
Repository: ambari
Description
-------
When specifying a host predicate via the "host_predicate" property in the cluster creation template, the predicate is compiled using the API predicate compiler which will report syntactic errors in the predicate but the property names aren't validated. So, if an invalid property name is specified in the predicate, the host group will never get matched to any host meaning that the host group will never be deployed.
This is critical that this is fixed for 2.1 as it will cause great frustration to anyone who attempts to provision or scale a cluster and specifies a host predicate with an invalid proper name.
When an invalid property name is specified a host predicate, this should result in a 400 response to the the user with a message which indicates the invalid properties.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 77f6d2c
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 47a4ce0
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java a1a0ac6
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java 1530a3e
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequestTest.java acfd426
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ScaleClusterRequestTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/34157/diff/
Testing
-------
Functional Testing:
Provisioned and scaled clusters with combination of explicit host names, host counts and host predicates and host registrations prior to and after cluster/scaling operations.
Unit Tests:
New Tests and all existing tests pass.
Results :
Tests run: 2978, Failures: 0, Errors: 0, Skipped: 20
...
Total run:739
Total errors:0
Total failures:0
Thanks,
John Speidel
Re: Review Request 34157: Provide host predicate property validation
for blueprint requests
Posted by John Speidel <js...@hortonworks.com>.
> On May 13, 2015, 2:52 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java, lines 84-86
> > <https://reviews.apache.org/r/34157/diff/1/?file=957944#file957944line84>
> >
> > It seems like injection could be used rather than this `init` method. But this does match a pattern we see throughout the code.
Yeah, I often struggle with which 'injection' approach that I should use. Dependency Injection in Ambari is currently a real mess and is actually one of my "top 5" issues that I responded with regarding the recent "making things better poll". I left this like this for the moment because it is very likely that this "injection" wont be required in the near future as I am considering changing the TopologyRequest interface to return a blueprint name and not a blueprint.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34157/#review83610
-----------------------------------------------------------
On May 13, 2015, 3:41 a.m., John Speidel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34157/
> -----------------------------------------------------------
>
> (Updated May 13, 2015, 3:41 a.m.)
>
>
> Review request for Ambari, Robert Levas and Robert Nettleton.
>
>
> Bugs: AMBARI-11093
> https://issues.apache.org/jira/browse/AMBARI-11093
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When specifying a host predicate via the "host_predicate" property in the cluster creation template, the predicate is compiled using the API predicate compiler which will report syntactic errors in the predicate but the property names aren't validated. So, if an invalid property name is specified in the predicate, the host group will never get matched to any host meaning that the host group will never be deployed.
> This is critical that this is fixed for 2.1 as it will cause great frustration to anyone who attempts to provision or scale a cluster and specifies a host predicate with an invalid proper name.
> When an invalid property name is specified a host predicate, this should result in a 400 response to the the user with a message which indicates the invalid properties.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 77f6d2c
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 47a4ce0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java a1a0ac6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java 1530a3e
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequestTest.java acfd426
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ScaleClusterRequestTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34157/diff/
>
>
> Testing
> -------
>
> Functional Testing:
> Provisioned and scaled clusters with combination of explicit host names, host counts and host predicates and host registrations prior to and after cluster/scaling operations.
>
> Unit Tests:
> New Tests and all existing tests pass.
>
> Results :
>
> Tests run: 2978, Failures: 0, Errors: 0, Skipped: 20
> ...
> Total run:739
> Total errors:0
> Total failures:0
>
>
> Thanks,
>
> John Speidel
>
>
Re: Review Request 34157: Provide host predicate property validation
for blueprint requests
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34157/#review83610
-----------------------------------------------------------
Ship it!
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java
<https://reviews.apache.org/r/34157/#comment134621>
It seems like injection could be used rather than this `init` method. But this does match a pattern we see throughout the code.
- Robert Levas
On May 12, 2015, 11:41 p.m., John Speidel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34157/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 11:41 p.m.)
>
>
> Review request for Ambari, Robert Levas and Robert Nettleton.
>
>
> Bugs: AMBARI-11093
> https://issues.apache.org/jira/browse/AMBARI-11093
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When specifying a host predicate via the "host_predicate" property in the cluster creation template, the predicate is compiled using the API predicate compiler which will report syntactic errors in the predicate but the property names aren't validated. So, if an invalid property name is specified in the predicate, the host group will never get matched to any host meaning that the host group will never be deployed.
> This is critical that this is fixed for 2.1 as it will cause great frustration to anyone who attempts to provision or scale a cluster and specifies a host predicate with an invalid proper name.
> When an invalid property name is specified a host predicate, this should result in a 400 response to the the user with a message which indicates the invalid properties.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 77f6d2c
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 47a4ce0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java a1a0ac6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java 1530a3e
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequestTest.java acfd426
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ScaleClusterRequestTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34157/diff/
>
>
> Testing
> -------
>
> Functional Testing:
> Provisioned and scaled clusters with combination of explicit host names, host counts and host predicates and host registrations prior to and after cluster/scaling operations.
>
> Unit Tests:
> New Tests and all existing tests pass.
>
> Results :
>
> Tests run: 2978, Failures: 0, Errors: 0, Skipped: 20
> ...
> Total run:739
> Total errors:0
> Total failures:0
>
>
> Thanks,
>
> John Speidel
>
>
Re: Review Request 34157: Provide host predicate property validation
for blueprint requests
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34157/#review83621
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Nettleton
On May 13, 2015, 3:41 a.m., John Speidel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34157/
> -----------------------------------------------------------
>
> (Updated May 13, 2015, 3:41 a.m.)
>
>
> Review request for Ambari, Robert Levas and Robert Nettleton.
>
>
> Bugs: AMBARI-11093
> https://issues.apache.org/jira/browse/AMBARI-11093
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When specifying a host predicate via the "host_predicate" property in the cluster creation template, the predicate is compiled using the API predicate compiler which will report syntactic errors in the predicate but the property names aren't validated. So, if an invalid property name is specified in the predicate, the host group will never get matched to any host meaning that the host group will never be deployed.
> This is critical that this is fixed for 2.1 as it will cause great frustration to anyone who attempts to provision or scale a cluster and specifies a host predicate with an invalid proper name.
> When an invalid property name is specified a host predicate, this should result in a 400 response to the the user with a message which indicates the invalid properties.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 77f6d2c
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseClusterRequest.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 47a4ce0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java a1a0ac6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java 1530a3e
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequestTest.java acfd426
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ScaleClusterRequestTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34157/diff/
>
>
> Testing
> -------
>
> Functional Testing:
> Provisioned and scaled clusters with combination of explicit host names, host counts and host predicates and host registrations prior to and after cluster/scaling operations.
>
> Unit Tests:
> New Tests and all existing tests pass.
>
> Results :
>
> Tests run: 2978, Failures: 0, Errors: 0, Skipped: 20
> ...
> Total run:739
> Total errors:0
> Total failures:0
>
>
> Thanks,
>
> John Speidel
>
>