You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by twalthr <gi...@git.apache.org> on 2017/10/16 13:32:37 UTC
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
GitHub user twalthr opened a pull request:
https://github.com/apache/flink/pull/4837
[FLINK-6173] [table] Clean-up flink-table jar and dependencies
## What is the purpose of the change
This PR cleans the `flink-table` dependencies almost entirely. The jar only contains files that are important. It shades everything except for Janino, because this causes problems. I think this PR makes the dependency management for users easier. The resulting jar looks like:
<img width="514" alt="screen shot 2017-10-16 at 3 24 24 pm" src="https://user-images.githubusercontent.com/5746567/31614275-27bd5c74-b286-11e7-9d7b-113f6c4f7585.png">
I ran some smaller test programs and could not spot any problems, feel free to test it as well.
## Brief change log
Modification of the `flink-table` `pom.xml`.
## Verifying this change
Run all tests and run an example program on the cluster.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): yes
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no)
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/twalthr/flink FLINK-6173
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4837.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4837
----
commit e88670e6447cf3f3965222078ddc62b85fcb849c
Author: twalthr <tw...@apache.org>
Date: 2017-10-11T10:26:07Z
[FLINK-6173] [table] Clean-up flink-table jar and dependencies
----
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r145073241
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -252,6 +261,29 @@ under the License.
<pattern>org.eigenbase</pattern>
<shadedPattern>org.apache.flink.shaded.calcite.org.eigenbase</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>org.pentaho</pattern>
+ <shadedPattern>org.apache.flink.shaded.calcite.org.pentaho</shadedPattern>
+ </relocation>
+
+ <!-- flink-table dependencies -->
+ <relocation>
+ <pattern>org.apache.commons</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.apache.commons</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.reflections</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.reflections</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.joda.time</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.joda.time</shadedPattern>
+ </relocation>
+ <!-- not relocated for now -->
--- End diff --
no, but that's exactly what you should write as a comment into the pom ;)
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r144849032
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -252,6 +261,29 @@ under the License.
<pattern>org.eigenbase</pattern>
<shadedPattern>org.apache.flink.shaded.calcite.org.eigenbase</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>org.pentaho</pattern>
+ <shadedPattern>org.apache.flink.shaded.calcite.org.pentaho</shadedPattern>
+ </relocation>
+
+ <!-- flink-table dependencies -->
+ <relocation>
+ <pattern>org.apache.commons</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.apache.commons</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.reflections</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.reflections</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.joda.time</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.joda.time</shadedPattern>
+ </relocation>
+ <!-- not relocated for now -->
--- End diff --
why not?
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r144850032
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -213,36 +209,49 @@ under the License.
<filter>
<artifact>*:*</artifact>
<excludes>
+ <!-- excluded all these files for a clean flink-table jar -->
<exclude>org-apache-calcite-jdbc.properties</exclude>
+ <exclude>common.proto</exclude>
+ <exclude>requests.proto</exclude>
+ <exclude>responses.proto</exclude>
<exclude>mozilla/**</exclude>
<exclude>codegen/**</exclude>
<exclude>google/**</exclude>
<exclude>META-INF/*.SF</exclude>
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
+ <exclude>META-INF/services/**</exclude>
+ <exclude>properties.dtd</exclude>
+ <exclude>PropertyList-1.0.dtd</exclude>
+ <exclude>digesterRules.xml</exclude>
+ <!-- not excluded for now -->
+ <!-- <exclude>org.codehaus.commons.compiler.properties</exclude> -->
</excludes>
</filter>
</filters>
<artifactSet>
<includes combine.children="append">
+ <!-- Calcite and its dependencies -->
<include>org.apache.calcite:*</include>
<include>org.apache.calcite.avatica:*</include>
+ <include>com.google.guava:guava</include>
+ <include>org.eigenbase:*</include>
<include>net.hydromatic:*</include>
- <include>org.reflections:*</include>
+
+ <!-- flink-table dependencies -->
+ <include>commons-configuration:*</include>
+ <include>commons-lang:*</include>
+ <include>commons-codec:*</include>
<include>org.codehaus.janino:*</include>
- <include>com.google.guava:guava</include>
- </includes>
+ <include>org.reflections:*</include>
+ <include>joda-time:*</include>
+ </includes>
</artifactSet>
<relocations>
- <!-- We currently don't relocate slf4j as we have "logger not found"
- warnings otherwise during runtime -->
- <!--<relocation>
- <pattern>org.slf4j</pattern>
- <shadedPattern>org.apache.flink.shaded.calcite.org.slf4j</shadedPattern>
- </relocation>-->
+ <!-- Calcite and its dependencies -->
<relocation>
- <pattern>com.fasterxml.jackson</pattern>
- <shadedPattern>org.apache.flink.shaded.calcite.com.fasterxml.jackson</shadedPattern>
+ <pattern>org.apache.calcite</pattern>
+ <shadedPattern>org.apache.flink.shaded.calcite.org.apache.calcite</shadedPattern>
--- End diff --
For calcite dependencies, please change the shading pattern to org.apache.flink.calcite.shaded to a) make them more unique and b) distinguish them from flink-shaded dependencies.
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4837
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r144848665
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -252,6 +261,29 @@ under the License.
<pattern>org.eigenbase</pattern>
<shadedPattern>org.apache.flink.shaded.calcite.org.eigenbase</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>org.pentaho</pattern>
+ <shadedPattern>org.apache.flink.shaded.calcite.org.pentaho</shadedPattern>
+ </relocation>
+
+ <!-- flink-table dependencies -->
+ <relocation>
+ <pattern>org.apache.commons</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.apache.commons</shadedPattern>
--- End diff --
For flink dependencies, please change the shading pattern to `org.apache.flink.table.shaded` to a) make them more unique and b) distinguish them from flink-shaded dependencies.
---
[GitHub] flink issue #4837: [FLINK-6173] [table] Clean-up flink-table jar and depende...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/4837
I'll fix the broken build and will merge this PR.
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r144850901
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -213,36 +209,49 @@ under the License.
<filter>
<artifact>*:*</artifact>
<excludes>
+ <!-- excluded all these files for a clean flink-table jar -->
<exclude>org-apache-calcite-jdbc.properties</exclude>
+ <exclude>common.proto</exclude>
+ <exclude>requests.proto</exclude>
+ <exclude>responses.proto</exclude>
<exclude>mozilla/**</exclude>
<exclude>codegen/**</exclude>
<exclude>google/**</exclude>
<exclude>META-INF/*.SF</exclude>
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
+ <exclude>META-INF/services/**</exclude>
+ <exclude>properties.dtd</exclude>
+ <exclude>PropertyList-1.0.dtd</exclude>
+ <exclude>digesterRules.xml</exclude>
+ <!-- not excluded for now -->
--- End diff --
why not?
---
[GitHub] flink issue #4837: [FLINK-6173] [table] Clean-up flink-table jar and depende...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4837
the table examples no longer build on travis.
---
[GitHub] flink pull request #4837: [FLINK-6173] [table] Clean-up flink-table jar and ...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/4837#discussion_r144866086
--- Diff: flink-libraries/flink-table/pom.xml ---
@@ -252,6 +261,29 @@ under the License.
<pattern>org.eigenbase</pattern>
<shadedPattern>org.apache.flink.shaded.calcite.org.eigenbase</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>org.pentaho</pattern>
+ <shadedPattern>org.apache.flink.shaded.calcite.org.pentaho</shadedPattern>
+ </relocation>
+
+ <!-- flink-table dependencies -->
+ <relocation>
+ <pattern>org.apache.commons</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.apache.commons</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.reflections</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.reflections</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.joda.time</pattern>
+ <shadedPattern>org.apache.flink.shaded.org.joda.time</shadedPattern>
+ </relocation>
+ <!-- not relocated for now -->
--- End diff --
The problem is that Calcite is using the Janino properties file for determining the location of the compiler. We could relocate Janino as well but then we also have to change the contents of the properties file. Have you done this before?
---