You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "flyrain (via GitHub)" <gi...@apache.org> on 2026/04/03 00:02:24 UTC

[PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

flyrain opened a new pull request, #4116:
URL: https://github.com/apache/polaris/pull/4116

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)

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.

jbonofre commented on PR #4116:
URL: https://github.com/apache/polaris/pull/4116#issuecomment-4183974072

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain merged PR #4116:
URL: https://github.com/apache/polaris/pull/4116


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on PR #4116:
URL: https://github.com/apache/polaris/pull/4116#issuecomment-4183965611

   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`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain closed pull request #4116: Rename ExternalCatalogFactory to FederatedCatalogFactory
URL: https://github.com/apache/polaris/pull/4116


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Rename ExternalCatalogFactory to FederatedCatalogFactory [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.

jbonofre commented on code in PR #4116:
URL: https://github.com/apache/polaris/pull/4116#discussion_r3031767043


##########
CHANGELOG.md:
##########
@@ -41,7 +41,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 - The (Before/After)CommitViewEvent has been removed.
 - The (Before/After)CommitTableEvent has been removed.
 - The `PolarisMetricsReporter.reportMetric()` method signature has been extended to include a `receivedTimestamp` parameter of type `java.time.Instant`.
-- The `ExternalCatalogFactory.createCatalog()` and `createGenericCatalog()` method signatures have been extended to include a `catalogProperties` parameter of type `Map<String, String>` for passing through proxy and timeout settings to federated catalog HTTP clients.
+- The `ExternalCatalogFactory` interface has been renamed to `FederatedCatalogFactory`. The `createCatalog()` and `createGenericCatalog()` method signatures have been extended to include a `catalogProperties` parameter of type `Map<String, String>` for passing through proxy and timeout settings to federated catalog HTTP clients.

Review Comment:
   nit: it's a bit confusing to me. I would suggest:
   
   
   ```suggestion
   - The ExternalCatalogFactory interface has been renamed to FederatedCatalogFactory. Its createCatalog() and createGenericCatalog() method signatures have been extended to include a `catalogProperties` parameter of type `Map<String, String>` for passing through proxy and timeout settings to federated catalog HTTP clients.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org