Move CatalogConfigEndpointContributor to runtime/service

[dimas-b]

Following up on #4926:

• CatalogConfigEndpointContributor has no use cases in polaris-core.

• CatalogConfigEndpointContributor is effectively an SPI for the IRC implementation service code (it is not a "core" SPI)

• No changes to existing module dependencies.

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)

[flyrain]

LGTM overall. I'd love to have @flyingImer to take a look as well.

[flyingImer]

checking

[flyingImer]

Do we have an assessment of CatalogConfigEndpointContributor positioning? I usually go through below decision tree to determine where should a piece of code landing

Does this define a catalog ops related contract?
 |
 +-- No  -> internal detail, place it with the code that owns it
 |
 +-- Yes -> does it need more than one implementation?
       |
       +-- No (just a contract) -> a contract with one fixed home -> core/ or extensions/, place it with the code that owns it
       |
       +-- Yes (SPI) -> feature SPI (business domain like table CRUD) or substrate SPI (shared contract like Authz)?
             |
             +-- -> extensions/ if optional feature contract like osi, metrics
             |
             +-- -> core/

but I have a hard time here due to unclear how do we position it? I see it can be either be a SPI in core/ or some runtime adaptor layer specific internal details => runtime/

side note: I envision core/ to be runtime framework agnostic

hope it make sense

[dimas-b]

@flyingImer :

I see it can be either be a SPI in core/ [...]

Could you share some more details on this from your POV? As for me, I do not see how CatalogConfigEndpointContributor relates to polaris-core at all.

[flyingImer]

@flyingImer :

I see it can be either be a SPI in core/ [...]

Could you share some more details on this from your POV? As for me, I do not see how CatalogConfigEndpointContributor relates to polaris-core at all.

Oh I forgot to include that. So yea, if we see this interface being part of config feature spi (iceberg config rest spec), then it should land along with in core/

[dimas-b]

@flyingImer :

if we see this interface being part of config feature spi (iceberg config rest spec), then it should land along with in core/

Not quite. The Iceberg REST API is dealt with by the code in runtime/service. This is exactly why I'm moving CatalogConfigEndpointContributor there in this PR.

polaris-core deals with fundamental catalog aspects (entities, principals, authorization), but it is not specific to IRC (aside from minor edge cases). One could use polaris-core to implement a different API in principle.

[flyingImer]

@flyingImer :

if we see this interface being part of config feature spi (iceberg config rest spec), then it should land along with in core/

Not quite. The Iceberg REST API is dealt with by the code in runtime/service. This is exactly why I'm moving CatalogConfigEndpointContributor there in this PR.

polaris-core deals with fundamental catalog aspects (entities, principals, authorization), but it is not specific to IRC (aside from minor edge cases). One could use polaris-core to implement a different API in principle.

@dimas-b hmmm, it took me a bit to map your model to mine. I think both make sense.

You’re right that CatalogConfigEndpointContributor is REST-related. My point is: it’s still framework/runtime-agnostic.

I agree it’s not a great fit for core/, but runtime/ isn’t ideal either. runtime/ is very Quarkus-coupled, while this could still be reused in a Spring-based Polaris runtime.

I also noticed GenericTableEndpoints in core already has a TODO to move endpoint defs into a dedicated shared module. That module doesn’t exist yet, but PolarisRESTCatalog already needs it today: Spark client, no CDI, no Quarkus, importing from core for the same reason.

So we now have two consumers with the same need: IRC-specific, framework-agnostic code. core/ is too broad. runtime/ is too Quarkus-specific. Neither is the right long-term home.

Bottom line: no objection to this PR. runtime/ is fine for now. Longer term, this probably wants its own shared module: not core/, not runtime/. Separate issue, not this PR.

[dimas-b]

^ I re- requested a review from @flyingImer by mistake 😅

Longer term, this probably wants its own shared module: not core/, not runtime/. Separate issue, not this PR.

SGTM 👍 Merging "as is" for now. Open to future refactorings in this area.