apache / polaris

Apache Polaris, the interoperable, open source catalog for Apache Iceberg
https://polaris.apache.org/
Apache License 2.0
1.17k stars 130 forks source link

Refactor PolarisMetaStoreManager interface into multiple interfaces #417

Closed collado-mike closed 1 day ago

collado-mike commented 3 weeks ago

Description

PolarisMetaStoreManager is a monolithic interface that captures persistence, storage credential vending, remote cache, and grant management. This makes it easy to tie together many things that shouldn't be tied together - for example, the authorizer has dependencies on the cache types, so that it's impossible to make authorization pluggable using different grant rules.

This change strictly splits up the interface into multiple interfaces, but the single PolarisMetaStoreManager is still an extension of the others. Where it is possible to refer to specific interfaces, (e.g., the Resolver and the EntityCache), the references are changed, but we don't have factory types to generate realm-specific implementations of all interfaces and the PR was growing far too large when I started on that path.

Future PRs would update components to refer to specific interfaces and to break the tie between the PolarisMetaStoreManager interface and the others. The PolarisMetaStoreManagerImpl may continue to implement all interfaces.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Interface only change - all existing tests continue to pass.

Test Configuration:

Checklist:

Please delete options that are not relevant.

jbonofre commented 3 weeks ago

If it's generally a good idea, do you mind to hold this for a few days ? The reason that it would break the current Quarkus branch. I'm proposing:

  1. To open Quarkus draft PR to drive discussion/consensus
  2. Review this PR after 1. Thoughts ?
collado-mike commented 2 weeks ago

If it's generally a good idea, do you mind to hold this for a few days ? The reason that it would break the current Quarkus branch. I'm proposing:

  1. To open Quarkus draft PR to drive discussion/consensus
  2. Review this PR after 1. Thoughts ?

Hey @jbonofre - I have a branch at https://github.com/jbonofre/polaris/compare/QUARKUS...collado-mike:polaris:patch-1?expand=1 that has this change merged into your Quarkus branch. The code compiles, though when I run the tests locally, it fails due to missing AWS_REGION branch.

Can we merge that change into your branch and move forward with this refactoring in main?

collado-mike commented 2 days ago

Hey @jbonofre - I have a branch at https://github.com/jbonofre/polaris/compare/QUARKUS...collado-mike:polaris:patch-1?expand=1 that has this change merged into your Quarkus branch. The code compiles, though when I run the tests locally, it fails due to missing AWS_REGION branch.

Can we merge that change into your branch and move forward with this refactoring in main?

@jbonofre can we move forward with this PR?