Add Polaris Apprunner Gradle/Maven plugins #18

snazy · 2025-04-24T12:41:44Z

Gradle and Maven plugins to run a Polaris process and "properly" terminate it for integration testing.

More information in the README.md in the apprunner/ directory.

dimas-b · 2025-04-24T14:05:48Z

apprunner/codestyle/copyright-header-hash.txt

Could we share ASF stuff across all sub-modules?

flyrain

Thanks @snazy for the PR. This seems a huge one. I'd suggest a dev mailing discussion first. This plugin seems to be used for integration tests. Other than adding a whole plugin in to Polaris repo, is there any alternative? For example, is there any plugin can be imported directly instead of having all source code within the project.

flyrain · 2025-04-26T21:11:29Z

apprunner/README.md

+}
+```
+
+### Groovy DSL


Do we need groovy as Polaris is using Kotlin with Gradle?

We don't need Groovy as, indeed, Polaris is using Kotlin. However, I believe this project is a runner to facilitate Polaris users' life. Users can run the integration tests of their Enterprise projects in isolation without having to manually deploy a Polaris server themselves. And we cannot make any assumption as to whether they are using Groovy or Kotlin in their Gradle build.

flyrain · 2025-04-26T21:12:40Z

apprunner/README.md

+}
+```
+
+## Maven


Do we need maven doc as Polaris doesn't use maven?

Same comment as my previous answer: we don't need Maven as, indeed, Polaris is using Gradle.

However, I believe this project is a runner to facilitate Polaris users' life. Users can run the integration tests of their Enterprise projects in isolation without having to manually deploy a Polaris server themselves. Maven is the other mainstream Java build system. So it only makes sense that the Apprunner comes with instructions and code to support Maven too.

pingtimeout

No explicit approval yet as there are some questions I think would be worth answering. But generally speaking, the code looks good.

apprunner/README.md

pingtimeout · 2025-05-09T15:01:32Z

apprunner/README.md

+
+## FAQ
+
+### Does it have to be a Polaris Quarkus server?


Q: This FAQ entry makes me think that, eventually, the code from this module could be extracted and donated to Quarkus. Then this module would just become a specialization of a "Quarkus Gradle Runner for IT" project. Is this correct?

pingtimeout · 2025-05-09T15:05:06Z

apprunner/README.md

+}
+```
+
+### Groovy DSL


We don't need Groovy as, indeed, Polaris is using Kotlin. However, I believe this project is a runner to facilitate Polaris users' life. Users can run the integration tests of their Enterprise projects in isolation without having to manually deploy a Polaris server themselves. And we cannot make any assumption as to whether they are using Groovy or Kotlin in their Gradle build.

pingtimeout · 2025-05-09T15:06:45Z

apprunner/README.md

+}
+```
+
+## Maven


Same comment as my previous answer: we don't need Maven as, indeed, Polaris is using Gradle.

However, I believe this project is a runner to facilitate Polaris users' life. Users can run the integration tests of their Enterprise projects in isolation without having to manually deploy a Polaris server themselves. Maven is the other mainstream Java build system. So it only makes sense that the Apprunner comes with instructions and code to support Maven too.

pingtimeout · 2025-05-09T15:14:55Z

apprunner/common/src/main/java/org/apache/polaris/apprunner/common/InputBuffer.java

+
+      var any = false;
+      while (input.ready()) {
+        var c = input.read();


Reading form an input stream byte by byte could result in a lot of syscalls. It seems to me that using a BufferedInputStream would be a lot better. Am I missing something ? Or, is it acceptable because we only use the InputBuffer for a small amount of data?

It's only there to capture the startup phase output from Quarkus via its stdout.

pingtimeout · 2025-05-09T16:04:33Z

apprunner/common/src/main/java/org/apache/polaris/apprunner/common/ProcessHandler.java

+         * out as fast as possible. If there's no data available, the loop will "yield" via a
+         * Thread.sleep(1L), which is good enough.
+         *
+         * Note: we cannot do blocking-I/O here, because we have to read from both stdout+stderr.


Is this still true? Given that ProcessBuilder::redirectErrorStream was called, both stdout and stderr are merged into a single stream. So it seems to me that we could do blocking I/O.

Nope. It would block forever when the process doesn't yield any output.

pingtimeout · 2025-05-09T16:14:24Z

apprunner/common/src/main/java/org/apache/polaris/apprunner/common/ListenUrlWaiter.java

+  public void accept(String line) {
+    if (!listenUrl.isDone()) {
+      synchronized (capturedLog) {
+        capturedLog.add(line);


Is it possible that capturing all Quarkus output and keeping it in the heap could become a problem if, say, Polaris was started with TRACE logging? It seems to me that capturing all logs was mostly used for debugging purposes?

It's not a problem in practice. But you want to see all output in case the startup fails. So yes, it's for debugging when things don't work right w/ "your" server.

pingtimeout · 2025-05-09T16:51:52Z

...src/test/resources/org/apache/polaris/apprunner/plugin/TestSimulatingTestUsingThePlugin.java

+ */
+class TestSimulatingTestUsingThePlugin {
+  @Test
+  void pingNessie() throws Exception {


Suggested change

void pingNessie() throws Exception {

void pingPolaris() throws Exception {

Well, it's Nessie in this test.

pingtimeout · 2025-05-09T16:54:14Z

...src/test/resources/org/apache/polaris/apprunner/plugin/TestSimulatingTestUsingThePlugin.java

+
+    var client = NessieClientBuilder.createClientBuilderFromSystemSettings().withUri(uri).build(NessieApiV2.class);
+    // Just some simple REST request to verify that Polaris is started - no fancy interactions w/ Nessie
+    var config = client.getConfig();


There are references to Nessie that can most likely be simply renamed (e.g. pingNessie()) and others that I am less sure about (Nessie client usage). Is using the Nessie Client intended? There are mentions of Nessie's REST API in code that targets the embedded Polaris server, and that is confusing. What is the rationale for using the Nessie Client here?

At the time of pushing this, there was no Polaris binary.

However, it should eventually test both Polaris and Nessie. The principle (code wise) however is the same.

pingtimeout · 2025-05-09T16:56:44Z

.../src/test/java/org/apache/polaris/appruner/maven/mavenit/ITSimulatingTestUsingThePlugin.java

+    String uri = String.format("http://127.0.0.1:%s/api/v1", port);
+
+    NessieApiV1 client = HttpClientBuilder.builder().withUri(uri).build(NessieApiV1.class);
+    // Just some simple REST request to verify that Nessie is started - no fancy interactions w/ Nessie


More mentions of Nessie here

Co-authored-by: Pierre Laporte <[email protected]>

snazy · 2025-05-12T07:34:01Z

Heads up: this is the successor of apache/polaris#785 - it's been open for quite a while.

snazy requested review from adutra, ashvina, dennishuo, dimas-b, eric-maynard, jackye1995, jbonofre, vvcephei, collado-mike, RussellSpitzer, takidau, MonkeyCanCode, flyrain and ebyhr as code owners April 24, 2025 12:41

snazy force-pushed the apprunner branch 5 times, most recently from ae71aa5 to 6e7cdc1 Compare April 24, 2025 13:03

dimas-b approved these changes Apr 24, 2025

View reviewed changes

apprunner/codestyle/copyright-header-hash.txt Outdated

Copy link

dimas-b Apr 24, 2025

Choose a reason for hiding this comment

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

Could we share ASF stuff across all sub-modules?

snazy force-pushed the apprunner branch from 6e7cdc1 to f3d888b Compare April 26, 2025 18:36

flyrain requested changes Apr 26, 2025

View reviewed changes

Add Polaris Apprunner Gradle/Maven plugins

0eb0296

snazy force-pushed the apprunner branch from f3d888b to 0eb0296 Compare May 6, 2025 12:23

snazy requested a review from HonahX as a code owner May 6, 2025 12:23

pingtimeout reviewed May 9, 2025

View reviewed changes

Update apprunner/README.md

537ece1

Co-authored-by: Pierre Laporte <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Polaris Apprunner Gradle/Maven plugins #18

Add Polaris Apprunner Gradle/Maven plugins #18

snazy commented Apr 24, 2025

dimas-b Apr 24, 2025

flyrain left a comment

flyrain Apr 26, 2025

pingtimeout May 9, 2025

flyrain Apr 26, 2025

pingtimeout May 9, 2025

pingtimeout left a comment

pingtimeout May 9, 2025

pingtimeout May 9, 2025

pingtimeout May 9, 2025

pingtimeout May 9, 2025

snazy May 12, 2025 •

edited

Loading

pingtimeout May 9, 2025

snazy May 12, 2025

pingtimeout May 9, 2025

snazy May 12, 2025

pingtimeout May 9, 2025

snazy May 12, 2025

pingtimeout May 9, 2025

snazy May 12, 2025

snazy May 12, 2025

pingtimeout May 9, 2025

snazy commented May 12, 2025

	void pingNessie() throws Exception {
	void pingPolaris() throws Exception {

Add Polaris Apprunner Gradle/Maven plugins #18

Are you sure you want to change the base?

Add Polaris Apprunner Gradle/Maven plugins #18

Conversation

snazy commented Apr 24, 2025

Choose a reason for hiding this comment

flyrain left a 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

pingtimeout left a 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

snazy May 12, 2025 • 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

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

snazy commented May 12, 2025

snazy May 12, 2025 •

edited

Loading