Skip to content

[SPARK-24601][SPARK-27051][BACKPORT][CORE] Update to Jackson 2.9.8 #24418

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

Closed
wants to merge 2 commits into from

Conversation

srowen
Copy link
Member

@srowen srowen commented Apr 19, 2019

What changes were proposed in this pull request?

This backports:
ab1650d
7857c6d

which collectively updates Jackson to 2.9.8.

How was this patch tested?

Existing tests.

@srowen srowen self-assigned this Apr 19, 2019
@@ -224,6 +224,10 @@
<groupId>org.scala-lang</groupId>
<artifactId>scala-library</artifactId>
</dependency>
<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.

We used to get this transitively from Jackson; need to make it explicit now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this one is nasty, we've had soo many conflict on this one..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This much isn't a change to the final dependency graph, at least. It always need/depended on scala-reflect

@@ -36,7 +36,7 @@ import org.apache.spark.util.Utils
* (2) the Spark version of the client / server
* (3) an optional message
*/
@JsonInclude(Include.NON_NULL)
@JsonInclude(Include.NON_ABSENT)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the core of the change to maintain behavior, as NON_NULL changed behavior in 2.7 and 2.8

@SparkQA
Copy link

SparkQA commented Apr 19, 2019

Test build #104758 has finished for PR 24418 at commit 0bc8a95.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

cc @dbtsai

@arunmahadevan
Copy link
Contributor

LGTM

@dbtsai
Copy link
Member

dbtsai commented Apr 19, 2019

LGTM.

@SparkQA
Copy link

SparkQA commented Apr 19, 2019

Test build #104760 has finished for PR 24418 at commit 5c64255.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Merged to branch-2.4.

This is a collective backport (except 3.0-only ExecutorMetricsJsonDeserializer class).

Thank you, @srowen , @dbtsai , @felixcheung , @arunmahadevan , @Fokko !

dongjoon-hyun pushed a commit that referenced this pull request Apr 21, 2019
## What changes were proposed in this pull request?

This backports:
ab1650d
7857c6d

which collectively updates Jackson to 2.9.8.

## How was this patch tested?

Existing tests.

Closes #24418 from srowen/SPARK-24601.2.

Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@srowen
Copy link
Member Author

srowen commented Apr 21, 2019

I support this, and wanted to more fully explain the logic, as it occurs to me not all of it was visible in the conversation here:

  • The Jackson security issue has come up a few times on JIRAs
  • It was actually raised again on the security@ list for the project last week
  • I can also say this has been raised by a few customers at Databricks, FWIW
  • Having the older version is also beginning to cause problems with apps that want to include third-party libs that depend on newer Jackson
  • There's a perf boost

Weighing that against the relatively small behavior change I think it's the right thing for 2.4.x.

This won't go into 2.4.2 unless there is another RC. If not, it'll be in 2.4.3.

@srowen srowen deleted the SPARK-24601.2 branch April 22, 2019 00:52
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
## What changes were proposed in this pull request?

This backports:
apache@ab1650d
apache@7857c6d

which collectively updates Jackson to 2.9.8.

## How was this patch tested?

Existing tests.

Closes apache#24418 from srowen/SPARK-24601.2.

Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
## What changes were proposed in this pull request?

This backports:
apache@ab1650d
apache@7857c6d

which collectively updates Jackson to 2.9.8.

## How was this patch tested?

Existing tests.

Closes apache#24418 from srowen/SPARK-24601.2.

Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
## What changes were proposed in this pull request?

This backports:
apache@ab1650d
apache@7857c6d

which collectively updates Jackson to 2.9.8.

## How was this patch tested?

Existing tests.

Closes apache#24418 from srowen/SPARK-24601.2.

Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants