You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by uce <gi...@git.apache.org> on 2015/09/23 18:37:51 UTC
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
GitHub user uce opened a pull request:
https://github.com/apache/flink/pull/1174
[FLINK-2651] Add Netty to dependency management
Hadoop 2.7.0 pulls in an older Netty version, which clashes with our version. This makes sure to only pull in our version.
To test this, you have to build it with `-Dhadoop.version=2.7.0` set.
I've checked that this is respected in the fat jar.
Additionally, I've bumped the Netty version to the most recent 4.0.31 (from 4.0.27). There is no particular reason for this, but the release notes mention many improvements and fixes. In general, I think it's a good idea to have the most recent version.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/uce/flink netty-dep
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1174.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 #1174
----
commit f96cc13f2ef3336c87de0cd34bcbf3036b6c17e8
Author: Ufuk Celebi <uc...@apache.org>
Date: 2015-09-23T14:13:50Z
[FLINK-2651] Add Netty to dependency management
Hadoop 2.7.0 pulls in an older Netty version, which clashes with our version.
This makes sure to only pull in our version.
commit 371817101848fbb993c02e14a705b5399f754444
Author: Ufuk Celebi <uc...@apache.org>
Date: 2015-09-23T15:20:30Z
Bump Netty version to 4.0.31.Final
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142753142
@hsaputra I think the wiki page is not correct any more. Netty was discussed to be shaded, but was never added to the relocated artifacts.
That was a good decision, IMHO. Dependencies that upgrade well-behaved need not (and in most cases should not) be shaded. Netty behaved well so far: When the changed API and behavior, they used a different namespace. Same is true for the Apache Commons libraries, for example the switch from commons lang version 2 (in the old namespace) to commons lang version 3 (new package name and artifact id).
Unfortunately, same is not true for Guava, Joda Time, ASM, Protobuf, ...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142676424
OK, I will merge this then. Thanks for the reviews. I will remove the variable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142664975
Good fix.
I think it is a bit overkill to define a Maven variable for a dependency used only in one place, so why not directly state the version in the dependency management section?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142707891
Please update the license file in flink-dist accordingly.
On Sep 23, 2015 9:26 PM, "Robert Metzger" <no...@github.com> wrote:
> I agree. The build is working, for all hadoop versions, so I seems to work.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/flink/pull/1174#issuecomment-142704120>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142701270
I think the comments is wrong (I stumbled across it a while back as well). It came from the time when we were discussing to shade netty as well (which we don't do)...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1174
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142707617
But we do shade netty in the flink-shaded-hadoop module [1], don't we?
By explicit pulling specific Netty version I think it would just mean it will shade that particular version of Netty instead of the one from Hadoop. Or maybe I am wrong.
[1] https://cwiki.apache.org/confluence/display/FLINK/Hadoop+Versions+and+Dependency+Shading
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142700908
Let me check the implications tomorrow: I've put a comment there which explicitly says that putting netty there is dangerous ;)
```xml
<!-- this section defines the module versions that are used if nothing else is specified. -->
<dependencyManagement>
<!-- WARN:
DO NOT put guava,
protobuf,
asm,
netty
here. It will overwrite Hadoop's guava dependency (even though we handle it
separatly in the flink-shaded-hadoop module).
We can use all guava versions everywhere by adding it directly as a dependency to each project.
-->
<dependencies>
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142755444
That is good to know because I did similar thing in other project to make sure we have the right Netty version. But no shading involve either.
Agree about Guava for sure =(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142704120
I agree. The build is working, for all hadoop versions, so I seems to work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142714383
Adjusted LICENSE in bc21de2.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1174#issuecomment-142667072
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---