Skip to content

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

@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

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

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
sunchao pushed a commit to sunchao/iceberg that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants