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.

Sign up for GitHub

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

Jump to bottom

Build: relocate httpclient5 dependency for runtime jars #5761

Merged
merged 1 commit into from
Sep 16, 2022
Merged

Build: relocate httpclient5 dependency for runtime jars #5761

merged 1 commit into from
Sep 16, 2022

Conversation

ajantha-bhat
Copy link
Member

REST API catalog introduces a dependency on httpclient5 and it is included without proper relocation.
So, relocate httpclient5 dependency for runtime jars to avoid conflicts.

@ajantha-bhat
Copy link
Member Author

@pan3793 : Please take a look

@ajantha-bhat ajantha-bhat mentioned this pull request Sep 14, 2022
Merged
@pan3793
Copy link
Member

pan3793 commented Sep 14, 2022

The change LGTM, which part relies on httpclient5? better to verify the runtime jar works manually, because relocation is not always safe.

@ajantha-bhat ajantha-bhat marked this pull request as draft September 14, 2022 05:20
@ajantha-bhat ajantha-bhat marked this pull request as ready for review September 14, 2022 06:16
@ajantha-bhat
Copy link
Member Author

The change LGTM, which part relies on httpclient5? better to verify the runtime jar works manually, because relocation is not always safe.

@pan3793: As mentioned, REST catalog uses this httpclient5. REST catalog needs a server implementation(which is not present in the iceberg repo). I believe some of the tabular guys can verify this.
cc: @nastra, @Fokko, @kbendick

Fokko
Fokko approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Looks like we have all of them 👍🏻

grep -R "relocate 'org.threeten.extra'" .  | wc -l 
       9

pan3793
pan3793 approved these changes Sep 15, 2022
View reviewed changes
Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ajantha-bhat

@Fokko Fokko merged commit eff0700 into apache:master Sep 16, 2022
JonasJ-ap pushed a commit to JonasJ-ap/iceberg that referenced this pull request Sep 18, 2022
@ajantha-bhat @JonasJ-ap
Build: Relocate httpclient5 dependency for runtime jars (apache#5761)
96dd057
sunchao pushed a commit to sunchao/iceberg that referenced this pull request May 10, 2023
@ajantha-bhat @aokolnychyi
Build: Relocate httpclient5 dependency for runtime jars (apache#5761)
66ff34c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nastra nastra nastra approved these changes

@Fokko Fokko Fokko approved these changes

@pan3793 pan3793 pan3793 approved these changes

Assignees
No one assigned
Labels
build flink hive spark
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ajantha-bhat @pan3793 @nastra @Fokko