You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2017/05/04 15:07:57 UTC
Re: Review Request 58845: AMBARI-20891 - Allow extensions to
auto-link with supported stack versions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58845/#review173909
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
Line 5456 (original), 5457-5460 (patched)
<https://reviews.apache.org/r/58845/#comment246987>
Maybe use StringUtils.isNotBlank for these.
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 43 (patched)
<https://reviews.apache.org/r/58845/#comment246986>
This class can be made a Singleton and be injected as a dependency.
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 105 (patched)
<https://reviews.apache.org/r/58845/#comment246991>
Use {} notatation
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 111-112 (patched)
<https://reviews.apache.org/r/58845/#comment246988>
Use {} notatation
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 116 (patched)
<https://reviews.apache.org/r/58845/#comment246989>
Use {} notatation
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 123 (patched)
<https://reviews.apache.org/r/58845/#comment246990>
Use {} notatation
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 128 (patched)
<https://reviews.apache.org/r/58845/#comment246992>
Documentation
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
Lines 146-153 (patched)
<https://reviews.apache.org/r/58845/#comment246993>
Should this really be an exception if they are aleady linked, or can it be a information message with a NOOP?
ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java
Line 229 (original), 240-263 (patched)
<https://reviews.apache.org/r/58845/#comment246994>
Documentation on this. Also, what about the exception being logged if the extension could not be created. Should this be thrown as well?
- Jonathan Hurley
On April 28, 2017, 1:48 p.m., Tim Thorpe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58845/
> -----------------------------------------------------------
>
> (Updated April 28, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Jayush Luniya, and Nate Cole.
>
>
> Bugs: AMBARI-20891
> https://issues.apache.org/jira/browse/AMBARI-20891
>
>
> Repository: ambari
>
>
> Description
> -------
>
> It would possible to link extensions to supported stack versions while parsing the stacks, extensions and common-services directories.
>
> This would allow extensions to avoid making rest API calls to set up the link.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 751ce08
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ExtensionLinkDAO.java 3df1cd5
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ExtensionLinkEntity.java 12b3ce0
> ambari-server/src/main/java/org/apache/ambari/server/stack/ExtensionModule.java ce92ecf
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java 20e28fc
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java d3ad351
> ambari-server/src/main/java/org/apache/ambari/server/state/ExtensionInfo.java 656a8ce
> ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 353dd86
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/ExtensionMetainfoXml.java 77a7250
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerExtensionTest.java 9900eb5
> ambari-server/src/test/resources/extensions/EXT/0.1/metainfo.xml 77a832c
> ambari-server/src/test/resources/extensions/EXT/0.2/metainfo.xml 04f733c
> ambari-server/src/test/resources/extensions/EXT/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/themes/broken_theme.json PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/repos/repoinfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HBASE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/global.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hadoop-env.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hbase-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-log4j.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/package/dummy-script.py PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HIVE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/MAPREDUCE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/ZOOKEEPER/metainfo.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58845/diff/1/
>
>
> Testing
> -------
>
> mvn clean test
>
> mvn clean test -Dtest=StackManagerExtensionTest -DskipPythonTests
>
>
> Thanks,
>
> Tim Thorpe
>
>
Re: Review Request 58845: AMBARI-20891 - Allow extensions to
auto-link with supported stack versions
Posted by Tim Thorpe <tt...@ca.ibm.com>.
> On May 4, 2017, 3:07 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/58845/diff/1/?file=1703102#file1703102line43>
> >
> > This class can be made a Singleton and be injected as a dependency.
I was kind of debatting removing all the dao parameters to the StackManager constructor and move some of the code that uses them to the helper... Haven't done that so far.
- Tim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58845/#review173909
-----------------------------------------------------------
On April 28, 2017, 5:48 p.m., Tim Thorpe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58845/
> -----------------------------------------------------------
>
> (Updated April 28, 2017, 5:48 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Jayush Luniya, and Nate Cole.
>
>
> Bugs: AMBARI-20891
> https://issues.apache.org/jira/browse/AMBARI-20891
>
>
> Repository: ambari
>
>
> Description
> -------
>
> It would possible to link extensions to supported stack versions while parsing the stacks, extensions and common-services directories.
>
> This would allow extensions to avoid making rest API calls to set up the link.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 751ce08
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ExtensionLinkDAO.java 3df1cd5
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ExtensionLinkEntity.java 12b3ce0
> ambari-server/src/main/java/org/apache/ambari/server/stack/ExtensionModule.java ce92ecf
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java 20e28fc
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java d3ad351
> ambari-server/src/main/java/org/apache/ambari/server/state/ExtensionInfo.java 656a8ce
> ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 353dd86
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/ExtensionMetainfoXml.java 77a7250
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerExtensionTest.java 9900eb5
> ambari-server/src/test/resources/extensions/EXT/0.1/metainfo.xml 77a832c
> ambari-server/src/test/resources/extensions/EXT/0.2/metainfo.xml 04f733c
> ambari-server/src/test/resources/extensions/EXT/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/themes/broken_theme.json PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/repos/repoinfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HBASE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/global.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hadoop-env.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hbase-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-log4j.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/package/dummy-script.py PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HIVE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/MAPREDUCE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/ZOOKEEPER/metainfo.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58845/diff/1/
>
>
> Testing
> -------
>
> mvn clean test
>
> mvn clean test -Dtest=StackManagerExtensionTest -DskipPythonTests
>
>
> Thanks,
>
> Tim Thorpe
>
>
Re: Review Request 58845: AMBARI-20891 - Allow extensions to
auto-link with supported stack versions
Posted by Tim Thorpe <tt...@ca.ibm.com>.
> On May 4, 2017, 3:07 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java
> > Line 229 (original), 240-263 (patched)
> > <https://reviews.apache.org/r/58845/diff/1/?file=1703106#file1703106line240>
> >
> > Documentation on this. Also, what about the exception being logged if the extension could not be created. Should this be thrown as well?
During auto linking, I don't want to throw an exception because that would actually stop ambari server from starting.
- Tim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58845/#review173909
-----------------------------------------------------------
On April 28, 2017, 5:48 p.m., Tim Thorpe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58845/
> -----------------------------------------------------------
>
> (Updated April 28, 2017, 5:48 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Jayush Luniya, and Nate Cole.
>
>
> Bugs: AMBARI-20891
> https://issues.apache.org/jira/browse/AMBARI-20891
>
>
> Repository: ambari
>
>
> Description
> -------
>
> It would possible to link extensions to supported stack versions while parsing the stacks, extensions and common-services directories.
>
> This would allow extensions to avoid making rest API calls to set up the link.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 751ce08
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ExtensionLinkDAO.java 3df1cd5
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ExtensionLinkEntity.java 12b3ce0
> ambari-server/src/main/java/org/apache/ambari/server/stack/ExtensionModule.java ce92ecf
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java 20e28fc
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java d3ad351
> ambari-server/src/main/java/org/apache/ambari/server/state/ExtensionInfo.java 656a8ce
> ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 353dd86
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/ExtensionMetainfoXml.java 77a7250
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerExtensionTest.java 9900eb5
> ambari-server/src/test/resources/extensions/EXT/0.1/metainfo.xml 77a832c
> ambari-server/src/test/resources/extensions/EXT/0.2/metainfo.xml 04f733c
> ambari-server/src/test/resources/extensions/EXT/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/themes/broken_theme.json PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/repos/repoinfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HBASE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/global.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hadoop-env.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hbase-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-log4j.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/package/dummy-script.py PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HIVE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/MAPREDUCE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/ZOOKEEPER/metainfo.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58845/diff/1/
>
>
> Testing
> -------
>
> mvn clean test
>
> mvn clean test -Dtest=StackManagerExtensionTest -DskipPythonTests
>
>
> Thanks,
>
> Tim Thorpe
>
>
Re: Review Request 58845: AMBARI-20891 - Allow extensions to
auto-link with supported stack versions
Posted by Tim Thorpe <tt...@ca.ibm.com>.
> On May 4, 2017, 3:07 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java
> > Lines 146-153 (patched)
> > <https://reviews.apache.org/r/58845/diff/1/?file=1703102#file1703102line146>
> >
> > Should this really be an exception if they are aleady linked, or can it be a information message with a NOOP?
This method is called both for autolinking and in the REST API to link a specific extension version to a stack version. Removing the exception would change the REST API and make it look like the call actually worked.
The exception is caught and logged when autolinking.
- Tim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58845/#review173909
-----------------------------------------------------------
On April 28, 2017, 5:48 p.m., Tim Thorpe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58845/
> -----------------------------------------------------------
>
> (Updated April 28, 2017, 5:48 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Jayush Luniya, and Nate Cole.
>
>
> Bugs: AMBARI-20891
> https://issues.apache.org/jira/browse/AMBARI-20891
>
>
> Repository: ambari
>
>
> Description
> -------
>
> It would possible to link extensions to supported stack versions while parsing the stacks, extensions and common-services directories.
>
> This would allow extensions to avoid making rest API calls to set up the link.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 751ce08
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementHelper.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ExtensionLinkDAO.java 3df1cd5
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ExtensionLinkEntity.java 12b3ce0
> ambari-server/src/main/java/org/apache/ambari/server/stack/ExtensionModule.java ce92ecf
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackManager.java 20e28fc
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java d3ad351
> ambari-server/src/main/java/org/apache/ambari/server/state/ExtensionInfo.java 656a8ce
> ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 353dd86
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/ExtensionMetainfoXml.java 77a7250
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerExtensionTest.java 9900eb5
> ambari-server/src/test/resources/extensions/EXT/0.1/metainfo.xml 77a832c
> ambari-server/src/test/resources/extensions/EXT/0.2/metainfo.xml 04f733c
> ambari-server/src/test/resources/extensions/EXT/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/extensions/EXT/0.3/services/OOZIE2/themes/broken_theme.json PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/repos/repoinfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HBASE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/global.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hadoop-env.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hbase-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-log4j.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/configuration/hdfs-site.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HDFS/package/dummy-script.py PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/HIVE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/MAPREDUCE/metainfo.xml PRE-CREATION
> ambari-server/src/test/resources/stacks_with_extensions/HDP/0.3/services/ZOOKEEPER/metainfo.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58845/diff/1/
>
>
> Testing
> -------
>
> mvn clean test
>
> mvn clean test -Dtest=StackManagerExtensionTest -DskipPythonTests
>
>
> Thanks,
>
> Tim Thorpe
>
>