-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-3955] Different versions between jackson-mapper-asl and jackson-core-asl #2818
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
<version>${jackson.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.codehaus.jackson</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think defining a dependency here will have any effect if we don't ever use it in our own build... right? I don't think we list jackson-core-asl
anywhere in our component pom files as a dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pwendell Yes, that's true that that setting doesn't affect spark build, however, anyone use a jackson library in a spark framework, they always fail because of different versions between mapper and core of jackson. I don't think it's not nice and, in my opinion, there are two solutions. One is setting that pom.xml like that and another one is an exclusion of that library explicitly.
This setting does affect the build even though Spark does not use Jackson directly. It manages the version that third party deps see. Yes, they need to be harmonized and you get this unfortunate effect when one of two reps gets upgraded by a transitive dep but not the other. This does not get Jackson out of the user class path if that's what you mean? |
@srowen so is this for a case where user's aren't marking Spark as provided? |
@pwendell I think of it more as a change that would affect or help the assembly that one deploys on a cluster. It may, rather accidentally, affect user apps because this dep is not shaded. But I suppose if it's changing that for the better, it's still good, even if we'd prefer that user apps are shielded from the details of Spark's deps. |
Sorry I think I misinterpreted. So are you saying that by virtue of setting this version, our assembly jar will have different contents? I would think that unless this was declared somewhere in the |
Yes, that's what I mean. |
@pwendell @srowen I don't know dependencyManagement and shaded, but don't you think that's proper solution for fixing version? I also think that the shading is not the solution although that library is shaded because two depending libraries have different version. Actually, I don't understand what you are talking about, however, I think fixing version is needed. |
BTW I've run into similar problems with these dependencies on things that I was working on: Aside from the one being added here, I needed to also pull in (Note that commit is really old; adding this in |
I close this PR because of fixing it via #3379 |
Oops. I've misunderstood that SPARK-3955 has finished. I'm sorry and open another PR with same patch |
…n-c... ...ore-asl - set the same version to jackson-mapper-asl and jackson-core-asl - It's related with #2818 - coded a same patch from a latest master Author: Jongyoul Lee <[email protected]> Closes #3716 from jongyoul/SPARK-3955 and squashes the following commits: efa29aa [Jongyoul Lee] [SPARK-3955] Different versions between jackson-mapper-asl and jackson-core-asl - set the same version to jackson-mapper-asl and jackson-core-asl
…mapper-asl and jackson-core-asl pwendell 2483c1e didn't actually add a reference to `jackson-core-asl` as intended, but a second redundant reference to `jackson-mapper-asl`, as markhamstra picked up on (#3716 (comment)) This just rectifies the typo. I missed it as well; the original PR #2818 had it correct and I also didn't see the problem. Author: Sean Owen <[email protected]> Closes #3829 from srowen/SPARK-3955 and squashes the following commits: 6cfdc4e [Sean Owen] Actually refer to jackson-core-asl
Just add a specific version information about jackson-core-asl