tengqm commented on code in PR #5793:
URL: https://github.com/apache/gravitino/pull/5793#discussion_r1879129401
##########
api/src/main/java/org/apache/gravitino/SupportsCatalogs.java:
##########
@@ -98,6 +101,23 @@ Catalog createCatalog(
Map<String, String> properties)
throws NoSuchMetalakeException, CatalogAlreadyExistsException;
+ /**
+ * Create a managed catalog with specified catalog name, type, comment, and
properties.
+ *
+ * @param catalogName the name of the catalog.
+ * @param type the type of the catalog.
+ * @param comment the comment of the catalog.
+ * @param properties the properties of the catalog.
+ * @return The created catalog.
+ * @throws NoSuchMetalakeException If the metalake does not exist.
+ * @throws CatalogAlreadyExistsException If the catalog already exists.
+ */
+ default Catalog createCatalog(
+ String catalogName, Catalog.Type type, String comment, Map<String,
String> properties)
+ throws NoSuchMetalakeException, CatalogAlreadyExistsException {
+ return createCatalog(catalogName, type, null, comment, properties);
Review Comment:
```suggestion
String name, Catalog.Type type, String comment, Map<String, String>
properties)
throws NoSuchMetalakeException, CatalogAlreadyExistsException {
return createCatalog(name, type, null, comment, properties);
```
##########
api/src/main/java/org/apache/gravitino/credential/GCSTokenCredential.java:
##########
@@ -70,4 +83,11 @@ public Map<String, String> credentialInfo() {
public String token() {
return token;
}
+
+ private void validate(String token, long expireTimeInMs) {
Review Comment:
How about embed this method into `initialize`?
The GCSTokenCredential is immutable IIUC, so I suppose that this validation
would be invoked only from `initialize`.
We can move this out when this assumption doesn't hold in the future.
##########
clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java:
##########
@@ -858,6 +868,43 @@ public void accept(ErrorResponse errorResponse) {
}
}
+ /** Error handler specific to Credential operations. */
+ @SuppressWarnings("FormatStringAnnotation")
+ private static class CredentialErrorHandler extends RestErrorHandler {
+
+ private static final CredentialErrorHandler INSTANCE = new
CredentialErrorHandler();
+
+ @Override
+ public void accept(ErrorResponse errorResponse) {
+ String errorMessage = formatErrorMessage(errorResponse);
+
+ switch (errorResponse.getCode()) {
+ case ErrorConstants.ILLEGAL_ARGUMENTS_CODE:
+ throw new IllegalArgumentException(errorMessage);
+
+ case ErrorConstants.NOT_FOUND_CODE:
+ if
(errorResponse.getType().equals(NoSuchMetalakeException.class.getSimpleName()))
{
+ throw new NoSuchMetalakeException(errorMessage);
+ } else if (errorResponse
Review Comment:
Em ... I don't think we want to make this exception an explicit one.
The reason is that we may intentionally make this less clear, for security's
sake.
--
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]
