[
https://issues.apache.org/jira/browse/PHOENIX-2535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15232376#comment-15232376
]
Josh Elser commented on PHOENIX-2535:
-------------------------------------
(sorry, this is getting long. Do we have reviewboard or something set up going
forward?)
{code}
- <packaging>pom</packaging>
+ <packaging>jar</packaging>
{code}
{code}
+ <id>default-jar</id>
+ <phase>none</phase>
+ <goals/>
{code}
Why make this change in phoenix-assembly/pom.xml? It seems like you're just
working around Maven wanting to create a jar then. If you set this to {{pom}},
AFAIUI, that should not be trying to create any JAR for the module.
{code}
diff --git a/phoenix-assembly/src/build/components/all-common-jars.xml
b/phoenix-assembly/src/build/components/all-common-jars.xml
index 960c3c9..f8c5abd 100644
--- a/phoenix-assembly/src/build/components/all-common-jars.xml
+++ b/phoenix-assembly/src/build/components/all-common-jars.xml
@@ -24,12 +24,17 @@
<!-- Add the client & mapreduce jars. Expects the client jar packaging
phase to already be run,
which is determined by specification order in the pom. -->
<fileSet>
- <directory>target</directory>
+ <directory>${project.basedir}/../phoenix-client/target</directory>
<outputDirectory>/</outputDirectory>
<includes>
<include>phoenix-*-client.jar</include>
+ </includes>
+ </fileSet>
+ <fileSet>
+ <directory>${project.basedir}/../phoenix-server/target</directory>
+ <outputDirectory>/</outputDirectory>
+ <includes>
<include>phoenix-*-server.jar</include>
- <include>phoenix-*-mapreduce.jar</include>
</includes>
</fileSet>
<fileSet>
{code}
Is there a reason why you aren't putting JARs generated by a module within that
module's target/ directory? This is really confusing to see in practice "I
built this module. Where the heck are the artifacts?"
{code}
+ <relocation>
+ <pattern>com.codahale</pattern>
+
<shadedPattern>org.apache.phoenix.shaded.com.codahale</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+
<shadedPattern>org.apache.phoenix.shaded.com.fasterxml</shadedPattern>
+ </relocation>
{code}
Might be a bit more concise to make a property instead of repeating the shaded
relocation prefix. E.g.
{{<shaded.location>org.apache.phoenix.shaded</shaded.location>}} and then
{{<shadedPattern>$\{shaded.location\}.com.fasterxml</shadedPattern>}}. Is this
more concise?
{code}
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ </plugin>
+ </plugins>
+ </pluginManagement>
{code}
This looks unnecessary. The maven-shade-plugin should already be defined in
pluginManagement in {{/pom.xml}}.
{code}
+ <dependencies>
+ <!-- Depend on all other internal projects -->
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-core</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-flume</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-pig</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-spark</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-server</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.phoenix</groupId>
+ <artifactId>phoenix-server-client</artifactId>
+ </dependency>
+ </dependencies>
{code}
I'm surprised to see phoenix-server and phoenix-server-client in this list. My
initial thought was that phoenix-client would be a shaded jar for the thick
driver. Either way, neither thick or thin driver will need phoenix-server.
{code}
+ <module>phoenix-client</module>
{code}
Would it better to include the word "shaded" in the module name?
{code}
+ s
+ <transformer
{code}
A goof?
{code}
+ <transformers>
+ <transformer
+
implementation="org.apache.maven.plugins.shade.resource.IncludeResourceTransformer">
+ <resource>NOTICE</resource>
+ <file>${project.basedir}/../../NOTICE</file>
+ </transformer>
...
+ <transformer
+
implementation="org.apache.maven.plugins.shade.resource.IncludeResourceTransformer">
+ <resource>LICENSE.txt</resource>
+ <file>${project.basedir}/../../LICENSE.txt</file>
+ </transformer>
+ </transformers>
{code}
I hate to be the bearer of bad news, but these are most likely not meeting ASF
licensing requirements. For every bundled jar that Phoenix ships in a shaded
jar, we're going to have to
# Copy any NOTICE text into a NOTICE file in the shaded jar
# Copy necessary license and copyright information for non-ASLv2 licensed jars
into a LICENSE file in the shaded jar.
Yes, this will be horrible, but it is required.
> Create shaded clients (thin + thick)
> -------------------------------------
>
> Key: PHOENIX-2535
> URL: https://issues.apache.org/jira/browse/PHOENIX-2535
> Project: Phoenix
> Issue Type: Bug
> Reporter: Enis Soztutar
> Assignee: Sergey Soldatov
> Fix For: 4.8.0
>
> Attachments: PHOENIX-2535-1.patch, PHOENIX-2535-2.patch,
> PHOENIX-2535-3.patch, PHOENIX-2535-4.patch, PHOENIX-2535-5.patch
>
>
> Having shaded client artifacts helps greatly in minimizing the dependency
> conflicts at the run time. We are seeing more of Phoenix JDBC client being
> used in Storm topologies and other settings where guava versions become a
> problem.
> I think we can do a parallel artifact for the thick client with shaded
> dependencies and also using shaded hbase. For thin client, maybe shading
> should be the default since it is new?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
