Skip to content
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

Closed
wants to merge 3 commits into from
Closed

[SPARK-3955] Different versions between jackson-mapper-asl and jackson-core-asl #2818

wants to merge 3 commits into from

Conversation

jongyoul
Copy link
Member

Just add a specific version information about jackson-core-asl

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

1 similar comment
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

<version>${jackson.version}</version>
</dependency>
<dependency>
<groupId>org.codehaus.jackson</groupId>
Copy link
Contributor

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.

Copy link
Member Author

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.

@srowen
Copy link
Member

srowen commented Nov 10, 2014

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?

@pwendell
Copy link
Contributor

@srowen so is this for a case where user's aren't marking Spark as provided?

@srowen
Copy link
Member

srowen commented Nov 10, 2014

@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.

@pwendell
Copy link
Contributor

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 <dependencies> section of one of the Spark poms, it wouldn't matter.

@srowen
Copy link
Member

srowen commented Nov 12, 2014

Yes, that's what I mean. dependencyManagement is a way to manage transitive dependencies without actually making them into direct dependencies.

@jongyoul
Copy link
Member Author

@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.

@vanzin
Copy link
Contributor

vanzin commented Nov 18, 2014

BTW I've run into similar problems with these dependencies on things that I was working on:
vanzin@1f5db0e

Aside from the one being added here, I needed to also pull in jackson-xc and jackson-jaxrs, which were used by some Yarn APIs that I was trying out.

(Note that commit is really old; adding this in dependencyManagement and sticking to the Hadoop version instead of the Parquet version, like this PR does, is probably a better solution.)

@jongyoul
Copy link
Member Author

I close this PR because of fixing it via #3379

@jongyoul jongyoul closed this Dec 16, 2014
@jongyoul
Copy link
Member Author

Oops. I've misunderstood that SPARK-3955 has finished. I'm sorry and open another PR with same patch

asfgit pushed a commit that referenced this pull request Dec 27, 2014
…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 <jongyoul@gmail.com>

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
asfgit pushed a commit that referenced this pull request Dec 30, 2014
…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 <sowen@cloudera.com>

Closes #3829 from srowen/SPARK-3955 and squashes the following commits:

6cfdc4e [Sean Owen] Actually refer to jackson-core-asl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants