Skip to content

Move remaining build-time server properties to runtime/defaults#3341

Merged
dimas-b merged 1 commit intoapache:mainfrom
dimas-b:server-properties
Jan 5, 2026
Merged

Move remaining build-time server properties to runtime/defaults#3341
dimas-b merged 1 commit intoapache:mainfrom
dimas-b:server-properties

Conversation

@dimas-b
Copy link
Contributor

@dimas-b dimas-b commented Dec 30, 2025

The runtime/defaults module is meant as a holder of default Polaris Server runtime properties.

This change moves a few remaining properties from the application.properties file under runtime/server into runtime/defaults to avoid any possible confusion regarding the location of effective server properties.

Note: The Admin tool has its own application.properties file.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

The `runtime/defaults` module is meant as a holder of default
Polaris Server runtime properties.

This change moves a few remaining properties from the `application.properties`
file under `runtime/server` into `runtime/defaults` to avoid any possible
confusion regarding the location of effective server properties.

Note: The Admin tool has its own `application.properties` file.
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 5, 2026
@flyrain
Copy link
Contributor

flyrain commented Jan 5, 2026

Thanks for the PR, @dimas-b !

Would it be possible to move application.properties from runtime/defaults to runtime/server instead? That would align with the approach in the admin module, where the application.properties lives with the runnable modules.

I think this is to consolidate config files from both server and default modules to avoid conflicts. Question on potential user side conflicts, what happens if users add their own application.properties in the binary distribution or Docker image? Would it conflict with the one coming from runtime/defaults, and if so, which one wins?

@dimas-b
Copy link
Contributor Author

dimas-b commented Jan 5, 2026

Would it be possible to move application.properties from runtime/defaults to runtime/server instead?

Not in the current state of Polaris module. In other words, it'd be a much bigger refactoring. The polaris-runtime-defaults is reused by runtime/service.

I'd agree that the whole inter-module dependency graph is kind of convoluted at this time, but it's a different concern than disambiguating the source of application.properties.

@flyrain : If you prefer we can tackle the bigger dependency clean-up effort first. Yet, I thought that sorting out Quarkus properties would be a smaller change and might help in the bigger refactoring later.

@dimas-b
Copy link
Contributor Author

dimas-b commented Jan 5, 2026

Question on potential user side conflicts, what happens if users add their own application.properties in the binary distribution or Docker image?

Users should not meddle in jar contents. That's where the basic problem of configuration lookup exists (multiple application.properties files on the class path).

Using an external application.properties file has well-defined behaviour and is actually what we recommend in Polaris docs :) https://polaris.apache.org/in-dev/unreleased/configuration/

Downstream builds should not reuse polaris-runtime-defaults from Polaris (which is why it is a separate artifact).

@flyrain
Copy link
Contributor

flyrain commented Jan 5, 2026

Using an external application.properties file has well-defined behaviour and is actually what we recommend in Polaris docs :) https://polaris.apache.org/in-dev/unreleased/configuration/

That's good to know. Is the rule of thumb is to avoid multiple application.properties files packaged in Polaris jars?

@dimas-b
Copy link
Contributor Author

dimas-b commented Jan 5, 2026

Is the rule of thumb is to avoid multiple application.properties files packaged in Polaris jars?

In my experience it is preferable to have only one application.properties file (including transitive jar dependencies) in each Quarkus application.

Different Quarkus applications (including tests) can have other application.properties files in the same multi-module project, as long as those property files do not end up on the same class path.

@flyrain
Copy link
Contributor

flyrain commented Jan 5, 2026

In my experience it is preferable to have only one application.properties file (including transitive jar dependencies) in each Quarkus application.

I agreed that it seems messy if there are multiple application.properties files. Is there any Quarkus doc/issue to document this issue?

@dimas-b
Copy link
Contributor Author

dimas-b commented Jan 5, 2026

Is there any Quarkus doc/issue to document this issue?

None, to the best of my knowledge. Please note that "issues" in this case are very tricky to debug and may not always show up.... again, in my personal experience.

@dimas-b dimas-b merged commit 847f51e into apache:main Jan 5, 2026
15 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jan 5, 2026
@dimas-b dimas-b deleted the server-properties branch January 5, 2026 18:31
evindj pushed a commit to evindj/polaris that referenced this pull request Jan 26, 2026
…he#3341)

The `runtime/defaults` module is meant as a holder of default
Polaris Server runtime properties.

This change moves a few remaining properties from the `application.properties`
file under `runtime/server` into `runtime/defaults` to avoid any possible
confusion regarding the location of effective server properties.

Note: The Admin tool has its own `application.properties` file.
snazy added a commit to snazy/polaris that referenced this pull request Feb 11, 2026
* NoSQL: reduce heap pressure when running tests (apache#3267)

Some tests generate a lot of realms, likely one realm per test case. While the amount of data per realm is not much, it is nontheless nice to remove that data immediately (for tests).

The maintenance service, which purges data of eligible realms, cannot be run against the in-memory backend (different JVM).

This change adds a rather "test only" workaround to purge the realm data in the in-memory backend immediately.

* fix(deps): update dependency org.projectnessie.cel:cel-bom to v0.6.0 (apache#3356)

* chore: Suppress unchecked case from mock (apache#3322)

This is to fix javac `warning: [unchecked] unchecked conversion TriConsumer<String, TableIdentifier, MetricsReport> mockConsumer = mock(TriConsumer.class);`

* Add Polaris Console on the tools set (apache#3355)

* Move remaining build-time server properties to runtime/defaults (apache#3341)

The `runtime/defaults` module is meant as a holder of default
Polaris Server runtime properties.

This change moves a few remaining properties from the `application.properties`
file under `runtime/server` into `runtime/defaults` to avoid any possible
confusion regarding the location of effective server properties.

Note: The Admin tool has its own `application.properties` file.

* Deprecate untyped `RealmConfig.getConfig()` (apache#3323)

* `getConfig(String)` has a generic return type, but the call path that gets the value does not perform any type validation.

* Deprecate this method in favour of well-typed `getConfig(PolarisConfiguration)`

* Migrate the single use case in Polaris code to the well-typed method.

* fix(deps): update dependency io.smallrye.common:smallrye-common-annotation to v2.15.0 (apache#3365)

* Fix error handler parameters in TaskExecutorImpl (apache#3358)

Use correct exception variable inside `CompletableFuture.exceptionallyComposeAsync()`

* fix(deps): update dependency org.junit:junit-bom to v5.14.2 (apache#3363)

* fix(deps): update dependency com.gradleup.shadow:shadow-gradle-plugin to v9.3.1 (apache#3361)

* Propagate previous task exceptions as "suppressed" (apache#3367)

* Propagate previous task exceptions as "suppressed"

Task retries may fail in different ways in each attempt, however
only the last exception used to be exposed to the caller.

This change propagates exceptions from all previous tasks execution
attempts as "suppressed" exceptions chained to the final tasks
failure exception.

* Introduce idempotency records schema and Postgres-backed IdempotencyStore (apache#3205)

This PR adds the persistence foundation for REST idempotency in Polaris:
Defines an idempotency_records table (Postgres) with key, binding, liveness, and finalization fields.
Introduces a storage‑agnostic IdempotencyRecord model and IdempotencyStore SPI in polaris-core.
Implements PostgresIdempotencyStore in polaris-relational-jdbc

* fix(deps): update dependency ch.qos.logback:logback-classic to v1.5.24 (apache#3369)

* fix(deps): update immutables to v2.12.1 (apache#3368)

* Remove unnecessary version spec in jdbc persistence build file (apache#3373)

The testcontainers BOM is already included, so the version spec, which doesn't match the BOM version, is unnecessary.

* Fix typo (apache#3376)

* Cosmetic: sort lines in libs.versions.toml (apache#3133)

* fix(deps): update dependency com.github.dasniko:testcontainers-keycloak to v4.1.0 (apache#3375)

* Remove Admin tests from required_status_checks (apache#3370)

* fix(deps): update quarkus platform and group to v3.30.6 (apache#3374)

* [Bug] Fix a bug that causing error when setting `write.data.path` to be a subdirectory of the table location (apache#3371)

Currently, when updating write.data.path of the table to a subdir under the table location, it will fail the location overlap check. For example

spark-sql> ALTER TABLE tb1 SET TBLPROPERTIES (
  'write.data.path' = '<tableLocation>/alternative_data'
);

org.apache.iceberg.exceptions.ForbiddenException: Forbidden: Unable to create table at location 's3://<table_location>' because it conflicts with existing table or namespace at location 's3://<table_location>`
IcebergCatalog.validateNoLocationOverlap(...) constructs a virtual PolarisEntity for overlap checking, but it did not set the entity name. When fetching the siblings of the table, it fails to filter out itself and thus the check mistaken considered that the write.data.path conflict with the table's own base location. (isChildOf)

This PR fix the issue by adding name to the virtual PolarisEntity and add a unit and a integration test.

* Update release guide to reference the proposed vote e-mail (apache#3377)

Now that apache#3150 has been merged, a VOTE e-mail is pre-generated by the
third workflow so that there is less room for error.  The release guide
has been updated to reflect this.

There is no pre-generated Incubator vote e-mail as part of the third
workflow.  This is acceptable in that eventually, after graduation, that
would become unnecessary.  But also that it could cause confusion as
there would be two proposed e-mail bodies in the third workflow.
Recommendation is to let the release manager created the Incubator vote
e-mail manually.

* Last merged commit b324fc3

---------

Co-authored-by: Mend Renovate <bot@renovateapp.com>
Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com>
Co-authored-by: JB Onofré <jbonofre@apache.org>
Co-authored-by: Huaxin Gao <huaxin.gao11@gmail.com>
Co-authored-by: Yong Zheng <yongzheng0809@gmail.com>
Co-authored-by: Yufei Gu <yufei@apache.org>
Co-authored-by: Honah (Jonas) J. <honahx@apache.org>
Co-authored-by: Pierre Laporte <pierre@pingtimeout.fr>
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.

4 participants