open-api: Build runtime jar for test fixture #11279

ajantha-bhat · 2024-10-08T05:53:39Z

Since the REST catalog TCK is merged (#10908),
we can have a runtime jar from the test fixture which can be used for building the docker image of rest catalog adapter.

PR for docker image will be raised in the follow up (already working on it)

ajantha-bhat · 2024-10-08T05:54:13Z

build.gradle

+    }
+  }
+
+  jar {


It was an empty jar since no source code. Hence, disabled it.

ajantha-bhat · 2024-10-08T05:55:05Z

build.gradle

+  shadowJar {
+    archiveBaseName.set("iceberg-open-api-test-fixtures-runtime")
+    archiveClassifier.set(null)
+    configurations = [project.configurations.testFixturesShadowImplementation]


can't directly depend on testFixturesImplementation as it is not resolved. Hence using via custom testFixturesShadowImplementation

ajantha-bhat · 2024-10-08T05:55:47Z

build.gradle

+  }
+
+  shadowJar {
+    archiveBaseName.set("iceberg-open-api-test-fixtures-runtime")


Double check the naming. LGTM.

kevinjqliu

This is a great idea!

Should we also publish this jar to maven?

iceberg/deploy.gradle

Lines 75 to 77 in 208ab20

    
           } else if (isOpenApi) { 
        
             artifact testJar 
        
             artifact testFixturesJar

ajantha-bhat · 2024-10-08T16:08:45Z

@kevinjqliu: I wasn't sure because of #9871.

I can add it. If we agree that it is useful.

kevinjqliu · 2024-10-08T16:13:29Z

#9871 is superseded by #10908

See
https://github.com/apache/iceberg/pull/10908/files#diff-d11c4195b39c7a98f6f1ed9ab99d1845ca19d297574e92b836c95ff6aa6e1701L25-L29

kevinjqliu · 2024-10-08T16:15:59Z

I initially thought this was creating the jar for the TCK, but it looks like its for the RESTCatalogServer class instead. I wonder if it would be easier to work with if the class is moved out of the iceberg-open-api project

ajantha-bhat · 2024-10-08T16:45:18Z

I initially thought this was creating the jar for the TCK, but it looks like its for the RESTCatalogServer class instead. I wonder if it would be easier to work with if the class is moved out of the iceberg-open-api project

I think the current design of keeping them as test fixture make sense to me. As it is not a production code.

danielcweeks · 2024-10-09T17:58:52Z

build.gradle

+
+    // include the LICENSE and NOTICE files for the runtime Jar
+    from(projectDir) {
+      include 'LICENSE'


We're actually packaging up more in this runtime than what's identified in the top-level LICENSE/NOTICE files, so we need to make sure that everything is captured.

Do we have to do this manually or we use the standard tool for this?

I don't think we have a standard guidelines for this in "CONTRIBUTING.md". Can we add this?

Used https://github.com/jk1/Gradle-License-Report to generate License and manually updated the notice for them.

Please check again.

cc: @jbonofre

Just spot checking the NOTICE, but it doesn't include the Kite notice from the Iceberg NOTICE file. @bryanck had some way of generating the license/notice files that included transitive dependencies (which is required by ASF).

@bryanck: Could you please help me here? I tried the same plugin that you mentioned in your PR here. I used the configuration as testFixturesRuntimeClasspath, because it is a shadow jar for test fixture source sets.

Also, We need to add the standard way in "contributing.md" for new users to follow it.

I used the same plugin, but for both the notices and licenses. Also, I appended to the root license and notice files, the root notice has the Kite notice.

@ajantha-bhat I can generate them if you want, I created a custom renderer for the plugin. I'm hoping to contribute that at some point so this is automated.

I can also try manually appending the root files contents.

It only generates licenses, how did you generate the notice from that plugin?

It can generate both, but I added some logic to my custom renderer to dedupe the notices.

ajantha-bhat · 2024-10-14T14:58:59Z

Retriggering build because of connection error

Downloading https://services.gradle.org/distributions/gradle-8.10.2-bin.zip

Error: Exception in thread "main" java.io.IOException: Server returned HTTP response code: 500 for URL: https://github.com/gradle/gradle-distributions/releases/download/v8.10.2/gradle-8.10.2-bin.zip
at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:2011)
at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1602)
at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:223)
at org.gradle.wrapper.Install.forceFetch(SourceFile:2)
at org.gradle.wrapper.Install$1.call(SourceFile:8)
at org.gradle.wrapper.GradleWrapperMain.main(SourceFile:67)
Error: Process completed with exit code 1.

ajantha-bhat · 2024-10-15T10:48:03Z

Any more suggestions for this? PR looks to be ready.

Fokko · 2024-10-28T20:29:04Z

@ajantha-bhat First of all, thanks for working on this, and sorry for the late reply as I was on parental leave. Unfortunately, there are issues with this. I just ran the following to check what's being shipped:

./gradlew build -x test -x integrationTest

And noticed that we ship quite a bit in the fat jar:

ls -lah open-api/build/libs/iceberg-open-api-*
-rw-r--r--  1 fokko.driesprong  staff   6.0K Oct 28 21:12 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-javadoc.jar
-rw-r--r--  1 fokko.driesprong  staff   6.0K Oct 28 21:12 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-sources.jar
-rw-r--r--  1 fokko.driesprong  staff   9.5K Aug 27 20:18 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-test-fixtures.jar
-rw-r--r--  1 fokko.driesprong  staff    10K Oct 28 21:12 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-tests.jar
-rw-r--r--  1 fokko.driesprong  staff   109M Oct 28 21:12 open-api/build/libs/iceberg-open-api-test-fixtures-runtime-1.7.0-SNAPSHOT.jar

When looking into the runtime:

mkdir /tmp/vo
mv open-api/build/libs/iceberg-open-api-test-fixtures-runtime-1.7.0-SNAPSHOT.jar /tmp/vo/iceberg-open-api-test-fixtures-runtime-1.7.0-SNAPSHOT.jar
cd /tmp/vo
unzip iceberg-open-api-test-fixtures-runtime-1.7.0-SNAPSHOT.jar

And looking at the content, it looks like we ship a LOT, much more than mentioned in the LICENSE. Including at least one licenses that we cannot ship as it is part of the category X:

ls -lah | grep -i lgpl
-rw-r--r--   1 fokko.driesprong  wheel   7.5K Feb  1  1980 LGPL-3.0.txt

Also for the release we probably want to track down where this comes from.

ajantha-bhat · 2024-10-29T17:17:06Z

@bryanck,

I have cleaned up the dependencies
Used license-report plugin to generate the report (plugin included in this PR)
./gradlew :iceberg-open-api:generateLicenseReport and report in build/reports/dependency-license/index.html
Used root notice and license.

Now I need your render tool to format and append the notice and license to existing file.

Fokko · 2024-10-30T12:28:50Z

Thanks for fixing that issue @ajantha-bhat, this looks much better:

ls -lah open-api/build/libs/iceberg-open-api-*   
-rw-r--r--  1 fokko.driesprong  staff   6.0K Oct 30 12:54 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-javadoc.jar
-rw-r--r--  1 fokko.driesprong  staff   6.0K Oct 30 12:54 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-sources.jar
-rw-r--r--  1 fokko.driesprong  staff    10K Oct 30 12:54 open-api/build/libs/iceberg-open-api-1.7.0-SNAPSHOT-tests.jar
-rw-r--r--  1 fokko.driesprong  staff    49M Oct 30 13:26 open-api/build/libs/iceberg-open-api-test-fixtures-runtime-1.7.0-SNAPSHOT.jar

I did some digging around and didn't find any red flags. I did notice that we ship parts of Hadoop that we probably need (Filesystem, Configuration, etc), and the 3.3.6 has a known vulnerability, so good to bump that: #11428

jbonofre

The fix looks good. I've just asked about cloudera mention in the NOTICE.

open-api/NOTICE

Fokko · 2024-10-30T16:16:28Z

This looks good, let's move this forward, and wrap up anything pending in #11283

ajantha-bhat · 2024-10-31T11:36:34Z

@Fokko: Thanks for the review. We can merge this and I can rebase the docker PR.

ajantha-bhat · 2024-11-05T15:42:43Z

build.gradle

@@ -967,11 +970,9 @@ project(':iceberg-open-api') {
    testFixturesImplementation project(':iceberg-gcp')
    testFixturesImplementation project(':iceberg-azure')
    testFixturesImplementation(libs.hadoop3.common) {
-      exclude group: 'log4j'


Added because of the warning while brining up the runtime jar.

ajantha-bhat · 2024-11-05T15:43:53Z

build.gradle

      exclude group: 'org.slf4j'
      exclude group: 'ch.qos.reload4j'
      exclude group: 'org.apache.avro', module: 'avro'
-      exclude group: 'com.fasterxml.woodstox'


Added these because of the error while brining up the runtime jar that class not found.

ajantha-bhat · 2024-11-05T15:47:23Z

@danielcweeks, @Fokko, @bryanck: Please take a look at the PR again. Thanks.

Fokko · 2024-11-06T08:17:30Z

Thanks @ajantha-bhat This looks good, if there are dangling issues, we can wrap them up in #11283

Let's move this forward, thanks @kevinjqliu, @jbonofre, @bryanck and @danielcweeks for the review 🙌

ajantha-bhat requested review from danielcweeks and Fokko October 8, 2024 05:53

github-actions bot added the build label Oct 8, 2024

ajantha-bhat commented Oct 8, 2024

View reviewed changes

build.gradle

}

}

jar {

Copy link

Member Author

ajantha-bhat Oct 8, 2024

Choose a reason for hiding this comment

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

It was an empty jar since no source code. Hence, disabled it.

ajantha-bhat commented Oct 8, 2024

View reviewed changes

build.gradle

}

shadowJar {

archiveBaseName.set("iceberg-open-api-test-fixtures-runtime")

Copy link

Member Author

ajantha-bhat Oct 8, 2024

Choose a reason for hiding this comment

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

Double check the naming. LGTM.

ajantha-bhat marked this pull request as draft October 8, 2024 11:04

ajantha-bhat force-pushed the runtime branch from 31c0e53 to 19f16ac Compare October 8, 2024 11:33

ajantha-bhat marked this pull request as ready for review October 8, 2024 11:36

ajantha-bhat mentioned this pull request Oct 8, 2024

REST: Docker file for REST Fixture #11283

Merged

kevinjqliu reviewed Oct 8, 2024

View reviewed changes

github-actions bot added the INFRA label Oct 8, 2024

danielcweeks reviewed Oct 9, 2024

View reviewed changes

github-actions bot added the OPENAPI label Oct 14, 2024

ajantha-bhat force-pushed the runtime branch from 1824c8f to e14800b Compare October 14, 2024 14:39

ajantha-bhat closed this Oct 14, 2024

ajantha-bhat reopened this Oct 14, 2024

ajantha-bhat force-pushed the runtime branch from e14800b to 753bd5e Compare October 29, 2024 17:14

ajantha-bhat force-pushed the runtime branch from 753bd5e to fdfe6db Compare October 30, 2024 05:17

ajantha-bhat force-pushed the runtime branch from fdfe6db to 00e38e1 Compare October 30, 2024 12:32

jbonofre reviewed Oct 30, 2024

View reviewed changes

open-api/NOTICE Show resolved Hide resolved

ajantha-bhat force-pushed the runtime branch 2 times, most recently from 5f9028f to f0db322 Compare October 30, 2024 13:06

Fokko approved these changes Oct 30, 2024

View reviewed changes

ajantha-bhat marked this pull request as draft October 31, 2024 12:47

open-api: Build runtime jar for test fixture

0aa01f9

ajantha-bhat force-pushed the runtime branch from f0db322 to 0aa01f9 Compare November 5, 2024 15:41

ajantha-bhat commented Nov 5, 2024

View reviewed changes

ajantha-bhat marked this pull request as ready for review November 5, 2024 15:46

Fokko merged commit 2ffd3b0 into apache:main Nov 6, 2024
51 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

open-api: Build runtime jar for test fixture #11279

open-api: Build runtime jar for test fixture #11279

ajantha-bhat commented Oct 8, 2024

ajantha-bhat Oct 8, 2024

ajantha-bhat Oct 8, 2024

ajantha-bhat Oct 8, 2024

kevinjqliu left a comment

ajantha-bhat commented Oct 8, 2024

kevinjqliu commented Oct 8, 2024

kevinjqliu commented Oct 8, 2024

ajantha-bhat commented Oct 8, 2024

danielcweeks Oct 9, 2024

ajantha-bhat Oct 11, 2024

ajantha-bhat Oct 14, 2024

ajantha-bhat Oct 14, 2024

danielcweeks Oct 17, 2024 •

edited

Loading

ajantha-bhat Oct 17, 2024

bryanck Oct 17, 2024

bryanck Oct 17, 2024

ajantha-bhat Oct 17, 2024

bryanck Oct 17, 2024

ajantha-bhat commented Oct 14, 2024

ajantha-bhat commented Oct 15, 2024

Fokko commented Oct 28, 2024

ajantha-bhat commented Oct 29, 2024 •

edited

Loading

Fokko commented Oct 30, 2024 •

edited

Loading

jbonofre left a comment

Fokko commented Oct 30, 2024

ajantha-bhat commented Oct 31, 2024

ajantha-bhat Nov 5, 2024

ajantha-bhat Nov 5, 2024

ajantha-bhat commented Nov 5, 2024

Fokko commented Nov 6, 2024

+                  }
+                }
+                jar {

	} else if (isOpenApi) {
	artifact testJar
	artifact testFixturesJar

open-api: Build runtime jar for test fixture #11279

open-api: Build runtime jar for test fixture #11279

Conversation

ajantha-bhat commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinjqliu left a comment

Choose a reason for hiding this comment

ajantha-bhat commented Oct 8, 2024

kevinjqliu commented Oct 8, 2024

kevinjqliu commented Oct 8, 2024

ajantha-bhat commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielcweeks Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajantha-bhat commented Oct 14, 2024

ajantha-bhat commented Oct 15, 2024

Fokko commented Oct 28, 2024

ajantha-bhat commented Oct 29, 2024 • edited Loading

Fokko commented Oct 30, 2024 • edited Loading

jbonofre left a comment

Choose a reason for hiding this comment

Fokko commented Oct 30, 2024

ajantha-bhat commented Oct 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajantha-bhat commented Nov 5, 2024

Fokko commented Nov 6, 2024

danielcweeks Oct 17, 2024 •

edited

Loading

ajantha-bhat commented Oct 29, 2024 •

edited

Loading

Fokko commented Oct 30, 2024 •

edited

Loading