Skip to content

Deprecate untyped RealmConfig.getConfig()#3323

Merged
dimas-b merged 1 commit intoapache:mainfrom
dimas-b:fix-config-type
Jan 5, 2026
Merged

Deprecate untyped RealmConfig.getConfig()#3323
dimas-b merged 1 commit intoapache:mainfrom
dimas-b:fix-config-type

Conversation

@dimas-b
Copy link
Contributor

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

  • 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.

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)

@dimas-b
Deprecate untyped RealmConfig.getConfig()
dc7eee1 
* `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.
@dimas-b dimas-b requested a review from HonahX December 23, 2025 19:30
@github-project-automation github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Dec 23, 2025
singhpk234
singhpk234 reviewed Dec 26, 2025
View reviewed changes
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfig.java
* @param <T> the type of the configuration value
* @param configName the name of the configuration key to check
* @return the current value set for the configuration key, or null if not set
* @deprecated Use typed {@link #getConfig(PolarisConfiguration)} instead.
Copy link
Contributor

@singhpk234 singhpk234 Dec 26, 2025

Choose a reason for hiding this comment

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

do we have a version in mind, where were wanna remove it, if we do it would be nice to add it here !

Copy link
Contributor Author

@dimas-b dimas-b Dec 29, 2025

Choose a reason for hiding this comment

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

I personally do not have a preference for a specific Polaris version where this can be removed, but open to suggestions :)

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 5, 2026
@dimas-b dimas-b requested a review from singhpk234 January 5, 2026 16:03
@dimas-b dimas-b merged commit 4cf09ec into apache:main Jan 5, 2026
15 checks passed
@dimas-b dimas-b deleted the fix-config-type branch January 5, 2026 18:51
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jan 5, 2026
evindj pushed a commit to evindj/polaris that referenced this pull request Jan 26, 2026
@dimas-b @evindj
Deprecate untyped RealmConfig.getConfig() (apache#3323)
f132ef0 
* `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.
snazy added a commit to snazy/polaris that referenced this pull request Feb 11, 2026
@snazy @renovate-bot
@dimas-b @jbonofre @huaxingao @MonkeyCanCode @flyrain @HonahX @pingtimeout
Dremio merge 2026 01 08 14 15 (apache#173)
bb56923 
* 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

Reviewers

@snazy snazy snazy approved these changes

@singhpk234 singhpk234 singhpk234 approved these changes

@HonahX HonahX Awaiting requested review from HonahX

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dimas-b @snazy @singhpk234