Skip to content

Simplify CatalogPrefixParser API#3622

Merged
adutra merged 1 commit intoapache:mainfrom
adutra:prefix-parser-refactor
Feb 5, 2026
Merged

Simplify CatalogPrefixParser API#3622
adutra merged 1 commit intoapache:mainfrom
adutra:prefix-parser-refactor

Conversation

@adutra
Copy link
Contributor

@adutra adutra commented Jan 30, 2026

This component's methods do not need to have a RealmContext parameter: either the implementation is application-scoped, in which case the realm is irrelevant; or the implementation is request-scoped, in which case it can have the RealmContext injected. (FWIW, the default implementation is application-scoped.)

This refactor will further simplify the remote request signing implementation.

Checklist

  • 🛡️ Don't disclose security issues! (contact [email protected])
  • 🔗 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)

Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Good point 👍

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 31, 2026
Copy link
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

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

This refactor seems reasonable to me, considering we have a path forward with injecting the realm-context in the impl.

i also would request @collado-mike @dennishuo also to take a look into this from the down stream service integration pov.

This component's methods do not need to have a `RealmContext` parameter: either the implementation is application-scoped, in which case the realm is irrelevant; or the implementation is request-scoped, in which case it can have the `RealmContext` injected. (FWIW, the default implementation is application-scoped.)

This refactor will further simplify the remote request signing implementation.
@adutra adutra force-pushed the prefix-parser-refactor branch from 6c2874b to 5c45d56 Compare February 4, 2026 18:57
@adutra
Copy link
Contributor Author

adutra commented Feb 4, 2026

@collado-mike @dennishuo any concerns? If not I propose to merge this on Feb. 5th.

Copy link
Contributor

@collado-mike collado-mike left a comment

Choose a reason for hiding this comment

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

I think this seems benign enough

Copy link
Contributor

@dennishuo dennishuo left a comment

Choose a reason for hiding this comment

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

LGTM

@adutra adutra merged commit a2418c8 into apache:main Feb 5, 2026
15 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Feb 5, 2026
@adutra adutra deleted the prefix-parser-refactor branch February 5, 2026 10:53
@adutra
Copy link
Contributor Author

adutra commented Feb 5, 2026

Thank you all for the reviews!

sungwy pushed a commit to sungwy/polaris that referenced this pull request Feb 7, 2026
This component's methods do not need to have a `RealmContext` parameter: either the implementation is application-scoped, in which case the realm is irrelevant; or the implementation is request-scoped, in which case it can have the `RealmContext` injected. (FWIW, the default implementation is application-scoped.)

This refactor will further simplify the remote request signing implementation.
snazy added a commit to snazy/polaris that referenced this pull request Feb 11, 2026
* Simplify CatalogPrefixParser API (apache#3622)

This component's methods do not need to have a `RealmContext` parameter: either the implementation is application-scoped, in which case the realm is irrelevant; or the implementation is request-scoped, in which case it can have the `RealmContext` injected. (FWIW, the default implementation is application-scoped.)

This refactor will further simplify the remote request signing implementation.

* Releasey: update check for required-checks (apache#3667)

After apache#3625 the `jq` select to look for required checks can be simplified to select only the `Required Checks` check/job.

* Wire external catalog properties into REST client config (apache#3480)

* pass ExternalCatalog.properties through federation factories (Iceberg REST, Hive, Hadoop)

* merge catalog properties with connection config, with connection config taking precedence

* document proxy/timeout settings for Iceberg REST federation

* add tests that exercise production merge logic

* Correct build instruction, project properties require org.gradle.project prefix (apache#3680)

* Correct build instruction, project properties require org.gradle.project prefix

---------

Co-authored-by: Nandor Kollar <[email protected]>

* Last merged commit 103c6ed

---------

Co-authored-by: Alexandre Dutra <[email protected]>
Co-authored-by: Yong-Jin Lee <[email protected]>
Co-authored-by: Nándor Kollár <[email protected]>
Co-authored-by: Nandor Kollar <[email protected]>
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.

5 participants