Skip to content

Rename ExternalCatalogFactory to FederatedCatalogFactory#4116

Merged
flyrain merged 2 commits intoapache:mainfrom
flyrain:rename2Federated
Apr 3, 2026
Merged

Rename ExternalCatalogFactory to FederatedCatalogFactory#4116
flyrain merged 2 commits intoapache:mainfrom
flyrain:rename2Federated

Conversation

@flyrain
Copy link
Copy Markdown
Contributor

@flyrain flyrain commented Apr 3, 2026

Rename ExternalCatalogFactory to FederatedCatalogFactory because "external catalog" is a broader concept than what this interface represents. An external catalog can be either:

  1. A federated catalog — Polaris connects to a remote catalog service (Iceberg REST, Hive, Hadoop) and delegates operations to it via a ConnectionConfigInfo.
  2. A notification-synced catalog — Polaris manages tables locally that are synced via the notification API, with no remote connection involved.

This factory interface only applies to the first case: it takes a ConnectionConfigInfoDpo and creates a remote catalog handle. Naming it ExternalCatalogFactory incorrectly implies
it covers all external catalog types. FederatedCatalogFactory makes the scope clear and is consistent with its existing implementations (HiveFederatedCatalogFactory,
HadoopFederatedCatalogFactory).

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
dimas-b previously approved these changes Apr 3, 2026
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Apr 3, 2026
jbonofre
jbonofre previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

I also suggest to rename createExternalCatalogFactory() method in AbstractIcebergCatalogHandlerAuthzTest.

Co-authored-by: JB Onofré <jbonofre@apache.org>
@flyrain flyrain dismissed stale reviews from jbonofre and dimas-b via ad37cab April 3, 2026 15:29
@flyrain
Copy link
Copy Markdown
Contributor Author

flyrain commented Apr 3, 2026

Thanks @dimas-b and @jbonofre for the review. Fixed the changelog per suggestion.

I also suggest to rename createExternalCatalogFactory() method in AbstractIcebergCatalogHandlerAuthzTest.

It is using PolarisCallContextCatalogFactory, which is unrelated to FederatedCatalog

@jbonofre
Copy link
Copy Markdown
Member

jbonofre commented Apr 3, 2026

@flyrain oh yes, sorry I was confused by the similar names 😄

@flyrain flyrain closed this Apr 3, 2026
@flyrain flyrain reopened this Apr 3, 2026
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Apr 3, 2026
@github-project-automation github-project-automation bot moved this from Done to PRs In Progress in Basic Kanban Board Apr 3, 2026
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Apr 3, 2026
@flyrain flyrain merged commit c36851a into apache:main Apr 3, 2026
34 of 36 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Apr 3, 2026
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.

3 participants