[FLINK-13748][S3][build] Fix jaxb relocation for S3.

[kl0u]

What is the purpose of the change

Fixes shading on Presto and Hadoop S3 FileSystems.

Brief change log

Changes the pom.xml in the flink-s3-fs-hadoop and flink-s3-fs-presto.

Verifying this change

This change is already covered by existing tests, such as the s3 e2e tests and can be verified by running the cron jobs on Travis.

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

• Dependencies (does it add or upgrade a dependency): (yes / no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
• The serializers: (yes / no / don't know)
• The runtime per-record code paths (performance sensitive): (yes / no / don't know)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / no / don't know)
• The S3 file system connector: (yes although its build process, not the logic / no / don't know)

Documentation

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

[flinkbot]

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.

Automated Checks

Last check on commit bc6537e (Wed Oct 16 08:24:31 UTC 2019)

Warnings:

• 2 pom.xml files were touched: Check for build and licensing issues.
• No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

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

[kl0u]

To reviewers: This is the build that verified that the change works: https://travis-ci.org/kl0u/flink/builds/585693473. As you can see something goes wrong during the building of the e2e-misc-jdk11 profile.

I will relaunch but at least the tests seem to work for java 8 as shown in https://api.travis-ci.org/v3/job/585693500/log.txt and https://api.travis-ci.org/v3/job/585693501/log.txt

[flinkbot]

CI report:

• bc6537e : SUCCESS Build

[tillrohrmann]

Thanks for opening this PR @kl0u. I was wondering whether we can't solve the problem a little bit easier. Would it be possible to simply add the dependency jaxb-api always to these two modules and relocate it?

[tillrohrmann]

Instead of adding this rather complicated shade snippet, why don't we always add the jaxb-api dependency via

<dependency>
    <groupId>javax.xml.bind</groupId>
	<artifactId>jaxb-api</artifactId>
	<version>2.3.0</version>
</dependency>

[tillrohrmann]

Same here. I think maintaining this shading logic can be quite a pain in the ass.

[kl0u]

@tillrohrmann I agree but adding by default the dependency seems to be failing to compile as shown here: https://travis-ci.org/kl0u/flink/builds/586431721 (this is the branch this travis run refers to https://github.com/kl0u/flink/tree/fix-jaxb-alt)

[kl0u]

It may be just a versioning issue. Java 8 needs jaxb 2.2.11. Let's see what Travis has to say.

[tillrohrmann]

@kl0u, would it work to add jaxb:2.2.11 as the dependency or does Java 11 require jaxb 2.3.x?

[kl0u]

@tillrohrmann I am running it on Travis to see

[StephanEwen]

We were planning to completely remove shading of the file systems for Flink 1.10 and put them into the plugins directory.

Would this be the time to do that? It seems a bit like wasted effort to fix shading when we want to remove it soon anyways.

[kl0u]

@StephanEwen I agree that if this is the goal, then this effort is "instant legacy".

That said, I do not feel comfortable doing the whole migration to the plugin mechanism for the filesystems myself. This is both due to time constraints, as I am already involved in other features, and because of non-familiarity with our plugin mechanism.

I can close this PR if you think that we should not pursue this solution any further.

[tillrohrmann]

If adding the jaxb dependency works, then I'd be in favour of doing it because it solves an immediate problem which are failing cron jobs. Moreover, this kind of fix should be a 5 liner. What is the status here @kl0u?

Once we get to removing shading, this fix then becomes obsolete and can be removed.

[kl0u]

The status can be seen here https://travis-ci.org/kl0u/flink/builds/586896522 and the branch with the changes where we only use jaxb 2.2.11 is here https://github.com/kl0u/flink/tree/fix-jaxb-alt-3 if you want to see what we are testing.