[FLINK-10984]Remove flink-shaded-hadoop from flink #8225

sunjincheng121 · 2019-04-19T10:36:02Z

What is the purpose of the change

To allow reasonable dependency management we should move flink-shaded-hadoop to flink-shaded, then Remove flink-shaded-hadoop from flink.

Brief change log

Remove flink-shaded-hadoop from flink.
Remove flink-shaded-hadoop/flink-shaded-hadoop2 from flink.
Move out flink-shaded-yran-test from flink-shaded-hadoop.
Modify the scripts in flink-dist, i.e. remove the logic about hadoop.
Modify some pom config about flink-shaded-hadoop.

Verifying this change

This change is already covered by existing tests, such as (please describe tests).

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)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (not documented)

flinkbot · 2019-04-19T10:37:02Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

sunjincheng121 · 2019-04-19T10:37:11Z

The CI will turn green after flink-shaded releases the 7.0 version.

flink-connectors/flink-connector-filesystem/pom.xml

flink-dist/pom.xml

pom.xml

sunjincheng121 · 2019-04-26T06:08:54Z

Thanks for your Review @zentol!

I have had updated the PR accordingly. And about download the uber JRA, I think your right! we should keep include-hadoop available. and we can download the jar by changing the hadoop.xml. i.e.: files ->dependencySet. and Rework flink-dist assemblies changes can in a new PR. What do you think?

Best,
Jincheng

zentol · 2019-05-02T11:37:15Z

pom.xml

@@ -53,7 +53,6 @@ under the License.
 		<!-- Dummy module to force execution of the Maven Shade plugin (see Shade plugin below) -->
 		<module>tools/force-shading</module>
 		<module>flink-annotations</module>
-		<module>flink-shaded-hadoop</module>


flink-shaded-yarn-tests must be added to this list

zentol · 2019-05-02T12:01:12Z

flink-dist/src/main/assemblies/hadoop.xml

-			<fileMode>0644</fileMode>
-		</file>
-	</files>
+			<useTransitiveDependencies>true</useTransitiveDependencies>


this is the default and can be omitted

zentol · 2019-05-02T12:05:21Z

flink-dist/src/main/assemblies/hadoop.xml

-		</file>
-	</files>
+			<useTransitiveDependencies>true</useTransitiveDependencies>
+			<scope>provided</scope>


we could add <outputFileNameMapping>${artifact.artifactId}-${artifact.baseVersion}.${artifact.extension}</outputFileNameMapping> , so that (for SNAPSHOT builds) the jar is always called flink-shaded-hadoop2-uber-2.4.1-1.9-SNAPSHOT.jar, and not flink-shaded-hadoop2-uber-2.4.1-1.9-629462874621478328407-23.jar

I think If we add the dependency for flink-shaded-hadoop-2-uber with <version>${hadoop.version}-${flink.shaded.version}</version>. we never using the SANAPSHOT JRA. e.g.: flink-shaded-hadoop-2-uber-2.4.1-7.0.jarIs there anything I misunderstand?

sunjincheng121 · 2019-05-31T06:06:22Z

The flink-shaded 7.0 already released, I'll rebase the code and let the CI passed.

zentol · 2019-06-05T09:46:02Z

pom.xml

@@ -121,7 +121,7 @@ under the License.
 		<chill.version>0.7.6</chill.version>
 		<zookeeper.version>3.4.10</zookeeper.version>
 		<curator.version>2.12.0</curator.version>
-		<jackson.version>2.7.9</jackson.version>
+		<jackson.version>2.9.8</jackson.version>


we're mixing things here. Let's open one PR that bumps the flink-shaded version and related dependencies, and migrate to hadoop separately.

Agree, So, we can merge this PR first, right? #8605

zentol · 2019-06-07T08:17:18Z

could you rebase the PR? I merged the flink-shaded version bump made in #8605.

zentol

There's still this weird pyflink-zip file in the PR.

zentol · 2019-06-07T08:19:02Z

flink-shaded-yarn-tests/pom.xml

@@ -146,42 +172,42 @@ under the License.
 							<relocations>
 								<relocation>
 									<pattern>com.google</pattern>
-									<shadedPattern>org.apache.flink.hadoop.shaded.com.google</shadedPattern>
+									<shadedPattern>org.apache.flink.hadoop2.shaded.com.google</shadedPattern>


I don't think we have to modify these.

Without this chanage, the test of YARNHighAvailabilityITCase cannot get success.

I'm currently looking into why that is the case. I'd very much prefer if this module would be independent of whatever hadoop version is in flink-dist.

…-hadoop from flink.

sunjincheng121 · 2019-06-09T19:36:07Z

Thanks for the Review @zentol
I have rebased the code and wait for CI change green.

sunjincheng121 · 2019-06-14T15:55:53Z

I appreciate if you can have another look :) @zentol

sunjincheng121 · 2019-06-18T08:45:40Z

Hi, @zentol Is there are any other comments from you? :)

sunjincheng121 changed the title ~~FLINK-10984][Build System, BuildSystem / Shaded~~ FLINK-10984]Remove flink-shaded-hadoop from flink Apr 19, 2019

rmetzger added the review=description? label Apr 19, 2019

rmetzger added component=BuildSystem component=BuildSystem/Shaded labels Apr 19, 2019

zentol requested changes Apr 24, 2019

View reviewed changes

flink-connectors/flink-connector-filesystem/pom.xml Outdated Show resolved Hide resolved

flink-dist/pom.xml Outdated Show resolved Hide resolved

flink-dist/pom.xml Show resolved Hide resolved

pom.xml Outdated Show resolved Hide resolved

zentol self-assigned this Apr 24, 2019

link3280 mentioned this pull request Apr 28, 2019

[FLINK-12356][BuildSystem] Optimise version experssion of flink-shaded-hadoop2(-uber) #8299

Closed

zentol reviewed May 2, 2019

View reviewed changes

sunjincheng121 changed the title ~~FLINK-10984]Remove flink-shaded-hadoop from flink~~ [FLINK-10984]Remove flink-shaded-hadoop from flink May 31, 2019

sunjincheng121 force-pushed the FLINK-10984-Part3 branch 2 times, most recently from 84dcc95 to e74f12c Compare May 31, 2019 09:29

zentol reviewed Jun 5, 2019

View reviewed changes

zentol reviewed Jun 7, 2019

View reviewed changes

[FLINK-10984][Build System, BuildSystem / Shaded] Remove flink-shaded…

c5305a5

…-hadoop from flink.

sunjincheng121 force-pushed the FLINK-10984-Part3 branch from 2333b34 to c5305a5 Compare June 9, 2019 19:34

zentol merged commit ee4a2f7 into apache:master Jun 19, 2019

yangjf2019 mentioned this pull request Sep 13, 2020

[hotfix][docs] Update shaded_version to the latest release #13370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-10984]Remove flink-shaded-hadoop from flink #8225

[FLINK-10984]Remove flink-shaded-hadoop from flink #8225

sunjincheng121 commented Apr 19, 2019

flinkbot commented Apr 19, 2019

sunjincheng121 commented Apr 19, 2019

sunjincheng121 commented Apr 26, 2019

zentol May 2, 2019

zentol May 2, 2019

zentol May 2, 2019

sunjincheng121 May 5, 2019

sunjincheng121 commented May 31, 2019

zentol Jun 5, 2019

sunjincheng121 Jun 5, 2019 •

edited

zentol commented Jun 7, 2019

zentol left a comment

zentol Jun 7, 2019

sunjincheng121 Jun 9, 2019

zentol Jun 19, 2019

sunjincheng121 commented Jun 9, 2019

sunjincheng121 commented Jun 14, 2019

sunjincheng121 commented Jun 18, 2019

[FLINK-10984]Remove flink-shaded-hadoop from flink #8225

[FLINK-10984]Remove flink-shaded-hadoop from flink #8225

Conversation

sunjincheng121 commented Apr 19, 2019

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Apr 19, 2019

Review Progress

sunjincheng121 commented Apr 19, 2019

sunjincheng121 commented Apr 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunjincheng121 commented May 31, 2019

Choose a reason for hiding this comment

sunjincheng121 Jun 5, 2019 • edited

Choose a reason for hiding this comment

zentol commented Jun 7, 2019

zentol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunjincheng121 commented Jun 9, 2019

sunjincheng121 commented Jun 14, 2019

sunjincheng121 commented Jun 18, 2019

sunjincheng121 Jun 5, 2019 •

edited