mchades commented on code in PR #12553:
URL: https://github.com/apache/gravitino/pull/12553#discussion_r3850597880
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java:
##########
@@ -951,24 +981,39 @@ public boolean dropSchema(NameIdentifier ident, boolean
cascade) throws NonEmpty
}
}
- /**
- * Since the Fileset catalog was completely managed by Gravitino, we don't
need to test the
- * connection
- *
- * @param catalogIdent the name of the catalog.
- * @param type the type of the catalog.
- * @param provider the provider of the catalog.
- * @param comment the comment of the catalog.
- * @param properties the properties of the catalog.
- */
@Override
- public void testConnection(
- NameIdentifier catalogIdent,
- Catalog.Type type,
- String provider,
- String comment,
- Map<String, String> properties) {
- // Do nothing
+ public void testConnection(NameIdentifier catalogIdent) {
+ if (disableFSOps) {
+ throw new UnsupportedOperationException(
+ "Fileset connection testing requires filesystem operations to be
enabled");
+ }
+ if (catalogStorageLocations.isEmpty()) {
+ throw new IllegalArgumentException("Fileset catalog has no catalog-level
location to test");
Review Comment:
Thanks for raising this. These cases are intentionally distinguished:
- `1006` means connection testing is unavailable for the catalog type or
configured operating mode, such as when Fileset filesystem operations are
disabled.
- `1001` means Fileset supports connection testing, but this catalog has no
catalog-level target in scope. A schema/fileset-only catalog remains valid;
this endpoint intentionally does not traverse child objects, and returning
success would incorrectly imply that a probe ran.
- `1007` means catalog-level targets were supplied, but the aggregate test
could not successfully probe all of them. Target-specific unsupported schemes
or providers are reported as `probe unsupported`; returning `1006` immediately
would skip the remaining locations, as discussed in the [earlier review
thread](https://github.com/apache/gravitino/pull/12553#discussion_r3841065873).
I’d prefer to keep the current behavior. I agree that the PR description
sentence is too broad and will clarify that `1006` applies when the catalog
type or configured operating mode does not support connection testing.
--
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]
