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-27029][BUILD] Update Thrift to 0.12.0 #23935
Conversation
Test build #102934 has finished for PR 23935 at commit
|
@@ -1945,7 +1944,7 @@ | |||
<dependency> | |||
<groupId>org.apache.thrift</groupId> | |||
<artifactId>libfb303</artifactId> | |||
<version>${libthrift.version}</version> | |||
<version>0.9.3</version> |
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.
it's ok this is 0.9.3?
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 mean mixin with libthrift 0.12
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.
Yeah, there's no newer version of libfb303. It seems to work though.
@@ -179,7 +179,6 @@ | |||
<joda.version>2.9.3</joda.version> | |||
<jodd.version>3.5.2</jodd.version> | |||
<jsr305.version>3.0.0</jsr305.version> | |||
<libthrift.version>0.9.3</libthrift.version> |
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.
@srowen . nit. Can we keep this? For example, jsr305.version
(at above line) is also used once but we have it.
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.
Sure that's fine. Normally I'd say it's just complicating the build but it also gives others a hook to change this version without maintaining a modification to the build file
Test build #102941 has finished for PR 23935 at commit
|
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.
+1, LGTM, too. Merged to master according to the target version on JIRA.
Thank you for this security fix, @srowen.
What changes were proposed in this pull request?
Update Thrift to 0.12.0 to pick up bug and security fixes.
Changes: https://github.com/apache/thrift/blob/master/CHANGES.md
The important one is for https://issues.apache.org/jira/browse/THRIFT-4506
How was this patch tested?
Existing tests. A quick local test suggests this works.