okumin commented on code in PR #5888:
URL: https://github.com/apache/hive/pull/5888#discussion_r2161315900
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -271,29 +241,10 @@ static String hmsCatalogMetricCount(String route) {
return HMS_METRIC_PREFIX + route.toLowerCase() + ".count";
}
- /**
- * @param apis an optional list of known api call names
- * @return the list of metric names for the HMSCatalog class
- */
- public static List<String> getMetricNames(String... apis) {
- final List<Route> routes;
- if (apis != null && apis.length > 0) {
- routes = Arrays.stream(apis)
- .map(HMSCatalogAdapter.Route::byName)
- .filter(Objects::nonNull)
- .collect(Collectors.toList());
- } else {
- routes = Arrays.asList(HMSCatalogAdapter.Route.values());
- }
- final List<String> metricNames = new ArrayList<>(routes.size());
- for (HMSCatalogAdapter.Route route : routes) {
- metricNames.add(hmsCatalogMetricCount(route.name()));
- }
- return metricNames;
- }
-
private ConfigResponse config() {
- return castResponse(ConfigResponse.class,
ConfigResponse.builder().build());
+ final List<Endpoint> endpoints = Arrays.stream(Route.values())
+ .map(r -> Endpoint.create(r.method.name(),
r.resourcePath)).collect(Collectors.toList());
+ return castResponse(ConfigResponse.class,
ConfigResponse.builder().withEndpoints(endpoints).build());
Review Comment:
This change is one of the key points of this PR. The Iceberg client doesn't
support views unless we explicitly declare that we support view endpoints.
https://github.com/apache/hive/pull/5887#issuecomment-2994195117
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -110,19 +112,18 @@ public class HMSCatalogAdapter implements RESTClient {
private static final String CLIENT_ID = "client_id";
private static final String ACTOR_TOKEN = "actor_token";
private static final String SUBJECT_TOKEN = "subject_token";
- private static final String VIEWS_PATH =
"v1/namespaces/{namespace}/views/{name}";
- private static final String TABLES_PATH =
"v1/namespaces/{namespace}/tables/{table}";
private final Catalog catalog;
private final SupportsNamespaces asNamespaceCatalog;
private final ViewCatalog asViewCatalog;
public HMSCatalogAdapter(Catalog catalog) {
+ Preconditions.checkArgument(catalog instanceof SupportsNamespaces);
+ Preconditions.checkArgument(catalog instanceof ViewCatalog);
this.catalog = catalog;
- this.asNamespaceCatalog =
- catalog instanceof SupportsNamespaces ? (SupportsNamespaces) catalog :
null;
- this.asViewCatalog = catalog instanceof ViewCatalog ? (ViewCatalog)
catalog : null;
+ this.asNamespaceCatalog = (SupportsNamespaces) catalog;
+ this.asViewCatalog = (ViewCatalog) catalog;
Review Comment:
I believe our adapter does not need to be so generic
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -133,82 +134,54 @@ enum HTTPMethod {
}
enum Route {
- TOKENS(HTTPMethod.POST, "v1/oauth/tokens",
- null, OAuthTokenResponse.class),
- SEPARATE_AUTH_TOKENS_URI(HTTPMethod.POST, "https://auth-server.com/token";,
- null, OAuthTokenResponse.class),
- CONFIG(HTTPMethod.GET, "v1/config",
- null, ConfigResponse.class),
- LIST_NAMESPACES(HTTPMethod.GET, "v1/namespaces",
- null, ListNamespacesResponse.class),
- CREATE_NAMESPACE(HTTPMethod.POST, "v1/namespaces",
- CreateNamespaceRequest.class, CreateNamespaceResponse.class),
- LOAD_NAMESPACE(HTTPMethod.GET, "v1/namespaces/{namespace}",
- null, GetNamespaceResponse.class),
- DROP_NAMESPACE(HTTPMethod.DELETE, "v1/namespaces/{namespace}"),
- UPDATE_NAMESPACE(HTTPMethod.POST, "v1/namespaces/{namespace}/properties",
- UpdateNamespacePropertiesRequest.class,
UpdateNamespacePropertiesResponse.class),
- LIST_TABLES(HTTPMethod.GET, "v1/namespaces/{namespace}/tables",
- null, ListTablesResponse.class),
- CREATE_TABLE(HTTPMethod.POST, "v1/namespaces/{namespace}/tables",
- CreateTableRequest.class, LoadTableResponse.class),
- LOAD_TABLE(HTTPMethod.GET, TABLES_PATH,
- null, LoadTableResponse.class),
- REGISTER_TABLE(HTTPMethod.POST, "v1/namespaces/{namespace}/register",
- RegisterTableRequest.class, LoadTableResponse.class),
- UPDATE_TABLE(HTTPMethod.POST, TABLES_PATH,
- UpdateTableRequest.class, LoadTableResponse.class),
- DROP_TABLE(HTTPMethod.DELETE, TABLES_PATH),
- RENAME_TABLE(HTTPMethod.POST, "v1/tables/rename",
- RenameTableRequest.class, null),
- REPORT_METRICS(HTTPMethod.POST,
"v1/namespaces/{namespace}/tables/{table}/metrics",
- ReportMetricsRequest.class, null),
- COMMIT_TRANSACTION(HTTPMethod.POST, "v1/transactions/commit",
- CommitTransactionRequest.class, null),
- LIST_VIEWS(HTTPMethod.GET, "v1/namespaces/{namespace}/views",
- null, ListTablesResponse.class),
- LOAD_VIEW(HTTPMethod.GET, VIEWS_PATH,
- null, LoadViewResponse.class),
- CREATE_VIEW(HTTPMethod.POST, "v1/namespaces/{namespace}/views",
- CreateViewRequest.class, LoadViewResponse.class),
- UPDATE_VIEW(HTTPMethod.POST, VIEWS_PATH,
- UpdateTableRequest.class, LoadViewResponse.class),
- RENAME_VIEW(HTTPMethod.POST, "v1/views/rename",
- RenameTableRequest.class, null),
- DROP_VIEW(HTTPMethod.DELETE, VIEWS_PATH);
+ TOKENS(HTTPMethod.POST, "v1/oauth/tokens", null),
Review Comment:
I copied & pasted 1.9.1 of the upstream and removed unused fields. When our
`/v1/config` returns the list of endpoints, the original form, such as
`v1/namespaces`, doesn't work. We need to be adaptable to the new form such as
`/v1/{prefix}/namespaces`.
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
