sunchao commented on a change in pull request #3385:
URL: https://github.com/apache/hadoop/pull/3385#discussion_r704988707



##########
File path: hadoop-client-modules/hadoop-client-integration-tests/pom.xml
##########
@@ -52,6 +52,16 @@
       <artifactId>junit</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>

Review comment:
       This doesn't look like the right approach, since the integration tests 
shouldn't depend on test dependency. As mentioned earlier, I think the issue is 
that the `snappy-java` is included in the optional dependency `hadoop-client` 
so it won't be a transitive dependency for downstream apps. Therefore, those 
apps will have to explicitly declare `snappy-java` as a dependency, which could 
cause surprises.
   
   Instead I think we should add `snappy-java` as compile dependency in 
`hadoop-client-api`. You may need to fight the maven enforcer rules for this.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to