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

[core] Fix missing kotlin and okio dependencies when using okhttp3. #5044

Closed

Conversation

LinMingQiang
Copy link
Contributor

1738921650869
1738921686999

Purpose

Linked issue: close #xxx

Tests

API and Format

Documentation

@@ -284,13 +298,23 @@ under the License.
<artifactSet>
<includes combine.children="append">
<include>com.squareup.okhttp3:okhttp</include>
<include>com.squareup.okio:okio-jvm</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please modify NOTICE file.

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 can't find the NOTICE file in okio-jvm

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will modify it later.

Copy link
Contributor

@jerry-024 jerry-024 Feb 10, 2025

Choose a reason for hiding this comment

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

you need add content in /paimon/paimon-core/src/main/resources/META-INF/NOTICE

This project bundles the following dependencies under the Apache Software License 2.0 (http://www.apache.org/licenses/LICENSE-2.0.txt)
- com.squareup.okio:okio:3.6.0

licence from: https://mvnrepository.com/artifact/com.squareup.okio/okio/3.6.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@jerry-024
Copy link
Contributor

The reason is okio‘s version so just add okio is ok.

<artifactId>okio-jvm</artifactId>
<version>${okio.version}</version>
</dependency>

Copy link
Contributor

Choose a reason for hiding this comment

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

        <dependency>
            <groupId>com.squareup.okio</groupId>
            <artifactId>okio</artifactId>
            <version>${okio.version}</version>
        </dependency>

just add this is ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I'll try it first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still : java.lang.ClassNotFoundException: kotlin.jvm.internal.Intrinsics

Now. I'll try shade okio.

        <dependency>
            <groupId>com.squareup.okio</groupId>
            <artifactId>okio</artifactId>
            <version>${okio.version}</version>
        </dependency>

just add this is ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean add okio, but still need to shade it. Because there is other version okio.

@LinMingQiang
Copy link
Contributor Author

The reason is okio‘s version so just add okio is ok.

I have tried, both okio and kotlin-stdlib need to be added.

@jerry-024
Copy link
Contributor

The reason is okio‘s version so just add okio is ok.

I have tried, both okio and kotlin-stdlib need to be added.

Can you write how to trigger this problem?

@LinMingQiang
Copy link
Contributor Author

LinMingQiang commented Feb 10, 2025

The reason is okio‘s version so just add okio is ok.

I have tried, both okio and kotlin-stdlib need to be added.

Can you write how to trigger this problem?

I used the partition mark done action of http-report. You need to package it into the development environment for testing instead of local testing.

@jerry-024
Copy link
Contributor

try this change

@LinMingQiang
Copy link
Contributor Author

LinMingQiang commented Feb 10, 2025

try this change

Still : ClassNotFoundException: kotlin.jvm.internal.Intrinsics.

After testing, kotlin-stdlib is required.

@LinMingQiang LinMingQiang force-pushed the master-fix-denpend-okhttp branch from fe419a1 to bf7a8f5 Compare February 10, 2025 07:31
@LinMingQiang LinMingQiang force-pushed the master-fix-denpend-okhttp branch from bf7a8f5 to b635741 Compare February 10, 2025 07:32
@jerry-024
Copy link
Contributor

I use the change when package it into the development environment. It works well.

@jerry-024
Copy link
Contributor

If you find kotlin-stdlib must need. According to okhttp's pom maybe we need use kotlin-stdlib-jdk8: 1.8.21 is better.

@LinMingQiang
Copy link
Contributor Author

LinMingQiang commented Feb 10, 2025

If you find kotlin-stdlib must need. According to okhttp's pom maybe we need use kotlin-stdlib-jdk8: 1.8.21 is better.

https://github.com/apache/paimon/actions/runs/13236825342/job/36943312581

I get java.lang.ClassNotFoundException: org.apache.paimon.shade.kotlin.jvm.internal.Intrinsics in the UT , the reason is okio-jvm and kotlin-stdlib will not be shaded into paimon-core, so i did not relocate okio-jvm and kotlin-stdlib in the paimon-core.

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.

3 participants