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-41800][BUILD] Upgrade commons-compress to 1.22 #39326

Closed
wants to merge 2 commits into from

Conversation

panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Dec 31, 2022

What changes were proposed in this pull request?

The pr aims to upgrade commons-compress from 1.21 to 1.22

Why are the changes needed?

This will bring the latest improvements.
image

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

@github-actions github-actions bot added the BUILD label Dec 31, 2022
@github-actions github-actions bot added the CORE label Dec 31, 2022
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</dependency>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

https://github.com/panbingkun/spark/actions/runs/3811706860/jobs/6485303553

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use CountingOutputStream class, so direct dependency should be added

import org.apache.commons.compress.utils.CountingOutputStream

Copy link
Member

Choose a reason for hiding this comment

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

Do you know why this is required by 1.22 only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will investigate root cause.

Copy link
Member

Choose a reason for hiding this comment

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

If this isn't just a breaking change in commons-compress, then I feel like we have two different versions of common-compress in play, which could explain why just adding the dependency explicitly changed behavior

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@panbingkun panbingkun changed the title [SPARK-41800][BUILD] Upgrade commons-compress to 1.22 [WIP][SPARK-41800][BUILD] Upgrade commons-compress to 1.22 Jan 5, 2023
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Looks good. It seems like we did need the explicit dependency for correctness anyway

@dongjoon-hyun dongjoon-hyun changed the title [WIP][SPARK-41800][BUILD] Upgrade commons-compress to 1.22 [SPARK-41800][BUILD] Upgrade commons-compress to 1.22 Jan 6, 2023
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 for Sean's comment.

@srowen
Copy link
Member

srowen commented Jan 6, 2023

Merged to master

@srowen srowen closed this in 60e87b6 Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants