Quarkus-based Polaris service runtime #469

adutra · 2024-11-22T18:36:46Z

Fixes #392.

Read the README_quarkus.md file for more details on what changes, how, and what remains to be done.

dimas-b · 2025-01-13T19:13:49Z

integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java

Noted! Change LGTM!

dropwizard/service/README-quarkus.md

dimas-b · 2025-01-13T19:57:00Z

...rc/test/java/org/apache/polaris/service/dropwizard/it/QuarkusRestCatalogIntegrationTest.java

nit: Do you think it should be @QuarkusIntegrationTest in the spirit of backbox tests?

Yes, but later: we don't have an intTest configuration in polaris yet :-)

Just add this to your build.gradle.kts (after #714)

testing { suites { val intTest by registering(JvmTestSuite::class) } } tasks.named("check") { dependsOn(testing.suites.named("intTest")) }

Nice, let's do it in a follow-up PR after the renamings etc.

When editing `.gitignore`, IntelliJ warns about a couple directories being ignored but not excluded from being indexed and searched. This change ignores the mentioned directories, except the one for DW, which is going away with apache#469.

Having documentation about the configuration that is both "up-to-date" and "in-sync with the source code" with the actual code is good practice. The functionality added in this change allows generating markdown docs from smallrye-config based configuration types, starting at and grouped by types that are annotated with `@ConfigMapping`. The javadoc of the config interfaces and properties is emitted as markdown. On top there is functionality to generate reference docs for "static" types (currently unused in Polaris). The code is nearly a 1:1 port of the same functionality in Nessie. This change will become useful with/after apache#469. Gradle projects added in this change are: * `polaris-config-doc-annotations` adds an annotation used when generating docs to influence the generation of the configuration reference docs. * `polaris-config-doc-generator` is the reference doc generator implementation, as a "standalone" tool that uses javadoc _infrastructure_. See `ReferenceConfigDocsGenerator` why it needs to be a separate tool than "just" a doclet (class loader isolation issues). * `polaris-site-reference-docs` actually generates the markdown files from referenced Gradle projects. Currently there are no projects referenced, so generation yields nothing. The generated docs are not yet integrated into the website, which will happen in a follow-up PR.

Having documentation about the configuration that is both "up-to-date" and "in-sync with the source code" with the actual code is good practice. The functionality added in this change allows generating markdown docs from smallrye-config based configuration types, starting at and grouped by types that are annotated with `@ConfigMapping`. The javadoc of the config interfaces and properties is emitted as markdown. On top there is functionality to generate reference docs for "static" types (currently unused in Polaris). The code is nearly a 1:1 port of the same functionality in Nessie. This change will become useful with/after #469. Gradle projects added in this change are: * `polaris-config-doc-annotations` adds an annotation used when generating docs to influence the generation of the configuration reference docs. * `polaris-config-doc-generator` is the reference doc generator implementation, as a "standalone" tool that uses javadoc _infrastructure_. See `ReferenceConfigDocsGenerator` why it needs to be a separate tool than "just" a doclet (class loader isolation issues). * `polaris-site-reference-docs` actually generates the markdown files from referenced Gradle projects. Currently there are no projects referenced, so generation yields nothing. The generated docs are not yet integrated into the website, which will happen in a follow-up PR.

flyrain · 2025-01-14T01:01:32Z

polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java

Nit: there are new comments in the caller, could we also add comments here to explain why we need cloned objects realmContext and polarisCallContext.

@flyrain this will soon disappear with #589 so I don't think there is need to explain.

But just for posterity, the reason is realted to TaskExecutor: these beans are request-scoped, and they would become unusable when the original request that created them completes, but the task hasn't finished executing yet.

flyrain · 2025-01-14T01:28:17Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

Question: are we going to use DI for the class PolarisAdminService later as well?

Probably. There is a huge overlap between PolarisServiceImpl (which is a request-scoped CDI bean) and PolarisAdminService, which is just a regular Pojo.

flyrain · 2025-01-14T01:31:29Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

I suggest postponing this change until we've fixed line 101.

Let's keep this PR as it is - there is already #463 to get rid of the problematic TLs

flyrain · 2025-01-14T01:31:58Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

I suggest postponing this change until we've fixed the line 101.

flyrain · 2025-01-14T01:44:20Z

service/common/build.gradle.kts

Considering this is a service-common module, do we need these libs in runtime classpaths?

I will take a note to check if these would work with compileOnly.

flyrain · 2025-01-14T02:24:35Z

dropwizard/service/README-quarkus.md

Can we improve the documentation to provide clearer and more detailed descriptions like the following:

Quarkus aggregates configuration properties from multiple sources, applying them in a specific order of precedence. When a property is defined in multiple sources, the value from the source with the higher priority overrides those from lower-priority sources. This behavior replaces the old configuration file format, such as polaris-server.yml.

The sources are listed below, from highest to lowest priority:

System properties: Properties set via the Java command line using -Dproperty.name=value.

Environment variables in .env file of the current working directory.

Settings in $PWD/config/application.properties file

The application configuration file packaged as part of the lib polaris-quarkus-service.

Default values: Hardcoded defaults within the application.

@flyrain this README is temporary, there is #700 to completely redesign the documentation.

There is already a paragraph about Quarkus configuration in #700, but I will make sure to include your suggestion above.

Incorporated the above changes in #700. FYI.

flyrain · 2025-01-14T02:51:44Z

dropwizard/service/README-quarkus.md

I believe this potentially breaks existing deployment as the endpoint url has changed, like this example shows:

"http://localhost:8182/healthcheck" --> "http://localhost:8182/q/health"

I'd recommend to keep the existing health check in this PR. Deprecated it later if needed. cc @dennishuo @collado-mike @eric-maynard

@flyrain if you really have an existing deployment and need the old endpoint, then you can add it to your build (as a system integrator you likely have your own build).

I don't think this is a reasonable response to proposed breaking changes. This is a bit like saying "if you have your own fork, you can patch it". It is a reasonable use case that someone would check out the Polaris repo, package it up in a Docker build and deploy it to their K8s application and use the provided health checker endpoint to monitor it. We wouldn't remove one of the management APIs out from under users and tell them to just fix it in their own build. I don't think the health checker or the metrics APIs ought to be treated any differently

We did not remove the health check endpoint nor the metrics endpoint.

@collado-mike : Does #782 help?

When editing `.gitignore`, IntelliJ warns about a couple directories being ignored but not excluded from being indexed and searched. This change ignores the mentioned directories, except the one for DW, which is going away with #469.

snazy · 2025-01-14T10:28:49Z

gradle/libs.versions.toml

 swagger = "1.6.14"

 [bundles]
+junit-testing = ["assertj-core", "mockito-core", "mockito-junit-jupiter", "junit-jupiter-api", "junit-jupiter-params", "junit-platform-reporting"]


I'll do a follow-up to adopt it to what #714 introduced

adutra force-pushed the polaris-quarkus-replacement branch from f714c40 to dcf8ccd Compare November 23, 2024 16:25

adutra mentioned this pull request Nov 25, 2024

Enhanced Apache Polaris runtime #392

Closed

adutra force-pushed the polaris-quarkus-replacement branch 4 times, most recently from 2f18194 to adfbbc2 Compare November 25, 2024 16:38

snazy mentioned this pull request Nov 25, 2024

Dynamic Config Resolver #459

Closed

8 tasks

adutra force-pushed the polaris-quarkus-replacement branch 7 times, most recently from 4ac2974 to 5cc6fec Compare November 28, 2024 10:42

adutra changed the title ~~[WIP] Quarkus-based Polaris service runtime~~ Quarkus-based Polaris service runtime Nov 28, 2024

adutra force-pushed the polaris-quarkus-replacement branch 13 times, most recently from fe8b9dc to 9fd4ce3 Compare November 29, 2024 16:16

snazy mentioned this pull request Dec 2, 2024

Working impl of HK2 dependency injection #493

Merged

11 tasks

snazy mentioned this pull request Dec 16, 2024

Fix LICENSE/LICENSE.bin for release #495

Merged

adutra force-pushed the polaris-quarkus-replacement branch 2 times, most recently from a4e8638 to ea66fd4 Compare January 13, 2025 19:08

dimas-b reviewed Jan 13, 2025

View reviewed changes

integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java Outdated

Copy link

Contributor

dimas-b Jan 13, 2025

Choose a reason for hiding this comment

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

Noted! Change LGTM!

eric-maynard reviewed Jan 13, 2025

View reviewed changes

dropwizard/service/README-quarkus.md Outdated Show resolved Hide resolved

dimas-b approved these changes Jan 13, 2025

View reviewed changes

snazy mentioned this pull request Jan 13, 2025

IntelliJ: exclude some directories from being indexed #720

Merged

adutra force-pushed the polaris-quarkus-replacement branch from ea66fd4 to c4e6ff0 Compare January 13, 2025 20:33

snazy mentioned this pull request Jan 13, 2025

Make tests work with Java 23 #723

Merged

flyrain reviewed Jan 14, 2025

View reviewed changes

Quarkus-based Polaris service runtime

e15f55a

adutra force-pushed the polaris-quarkus-replacement branch from c4e6ff0 to e15f55a Compare January 14, 2025 10:23

snazy reviewed Jan 14, 2025

View reviewed changes

snazy approved these changes Jan 14, 2025

View reviewed changes

adutra merged commit 4c7ae00 into apache:main Jan 14, 2025
5 checks passed

adutra deleted the polaris-quarkus-replacement branch January 14, 2025 10:38

This was referenced Jan 15, 2025

Dropwizard/HK2 service implementation doesn't bootstrap #565

Closed

Polaris not starting up / TaskHandlerConfiguration not JSON-serializable #561

Closed

adutra mentioned this pull request Jan 25, 2025

Copy RealmId when passing it to TaskExecutorImpl #879

Merged

singhpk234 mentioned this pull request Apr 1, 2025

[JDBC] Part2: Add Relational JDBC module #1287

Merged

4 tasks

Quarkus-based Polaris service runtime #469

Quarkus-based Polaris service runtime #469

Uh oh!

Conversation

adutra commented Nov 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

adutra commented Nov 22, 2024 •

edited

Loading