payert commented on a change in pull request #3305:
URL: https://github.com/apache/ambari/pull/3305#discussion_r617517794
##########
File path: ambari-metrics/ambari-metrics-assembly/pom.xml
##########
@@ -837,13 +894,43 @@
<data>
<src>${collector.dir}/target/embedded/${hbase.folder}</src>
<type>directory</type>
- <excludes>bin/**,bin/*,lib/*tests.jar</excludes>
+
<excludes>bin/**,bin/*,lib/*tests.jar,lib/hadoop*jar,lib/guava-11.0.2.jar,lib/commons-beanutils-core-1.8.0.jar</excludes>
Review comment:
Can we add line break after the comma for better readability ?
##########
File path: ambari-metrics/ambari-metrics-assembly/pom.xml
##########
@@ -266,10 +266,45 @@
<exclude>lib/*tests.jar</exclude>
<exclude>lib/findbugs*.jar</exclude>
<exclude>lib/jdk.tools*.jar</exclude>
+ <exclude>lib/hadoop*.jar</exclude>
Review comment:
I guess you are excluding here the jars from the hbase/lib that will be
replaced later, right? You can add a comment why are those excluded.
##########
File path: ambari-metrics/ambari-metrics-timelineservice/pom.xml
##########
@@ -738,6 +737,25 @@
<version>3.2</version>
<scope>test</scope>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
Review comment:
Think about reducing the scope of the dependencies maybe to 'provided'.
Also you could add comments to elaborate on why the dependencies are
required.
##########
File path: ambari-metrics/ambari-metrics-timelineservice/pom.xml
##########
@@ -738,6 +737,25 @@
<version>3.2</version>
<scope>test</scope>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-minicluster</artifactId>
+ <version>${hadoop.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-client</artifactId>
+ <version>${hadoop.version}</version>
+ </dependency>
+
+ <dependency>
Review comment:
The commons-beanutils-1.9.3.jar located in
ambari-metrics-assembly/target/embedded/hadoop-3.1.1/share/hadoop/common/lib/
Maybe we could use that instead of introducing a new managed dependency here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
