mchades commented on code in PR #12553:
URL: https://github.com/apache/gravitino/pull/12553#discussion_r3851237984
##########
core/src/main/java/org/apache/gravitino/connector/CatalogOperations.java:
##########
@@ -60,11 +60,24 @@ void initialize(
* @param properties the properties of the catalog.
* @throws Exception if connection fails.
*/
- void testConnection(
+ default void testConnection(
Review Comment:
> Nothing implements **only** the five-argument version...
This case is already covered, although not by a test named specifically for
default-method compatibility. `TestCatalogOperations` implements only the
legacy five-argument method. `TestCatalogManager#testCreateCatalog` verifies
that the pre-create path still invokes it successfully, while
`testEnableAndDisableCatalog` verifies that the new one-argument call uses the
default implementation and throws `UnsupportedOperationException`. The REST
tests separately verify that this maps to `1006`. I therefore do not think an
additional interface-level test is necessary.
> `core/src/main/java/org/apache/gravitino/catalog/SupportsCatalogs` gained
an abstract method...
The compatibility note refers specifically to the public Java API and the
connector `CatalogOperations` SPI; both new methods are default
implementations. The core `SupportsCatalogs` interface is an internal manager
contract rather than either of those extension surfaces. I agree the wording
can be clearer, so I will change it to:
> The new public Java API and connector `CatalogOperations` SPI methods use
default implementations, so this does not introduce a binary or source
incompatibility for existing implementations.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
