roryqi commented on code in PR #11280:
URL: https://github.com/apache/gravitino/pull/11280#discussion_r3654805376


##########
design-docs/spark-rest-catalog-registration.md:
##########
@@ -0,0 +1,360 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+-->
+
+# Design: Spark Lakehouse REST Catalog Automatic Registration for Apache 
Gravitino
+
+---
+
+## Background
+
+Connecting Spark to Gravitino takes little configuration:
+
+```text
+spark.plugins=org.apache.gravitino.spark.connector.plugin.GravitinoSparkPlugin
+spark.sql.gravitino.uri=http://127.0.0.1:8090
+spark.sql.gravitino.metalake=test
+```
+
+But accessing Iceberg tables through the Gravitino Iceberg REST server still 
requires hand-written
+configuration per catalog, duplicating what the REST server already manages 
and needing an edit
+whenever catalogs are added or removed:
+
+```text
+spark.sql.catalog.iceberg_prod=org.apache.iceberg.spark.SparkCatalog
+spark.sql.catalog.iceberg_prod.type=rest
+spark.sql.catalog.iceberg_prod.uri=http://127.0.0.1:9001/iceberg/
+spark.sql.catalog.iceberg_prod.warehouse=iceberg_prod
+```
+
+---
+
+## Goals
+
+1. **Automatic Iceberg registration**: A Spark session configured with only 
the new plugin and the
+   Iceberg REST server URI registers one Spark Iceberg REST catalog per 
catalog served by that
+   server, with no per-catalog configuration.
+2. **Server-authoritative catalog list**: The REST server tells Spark which 
catalogs it serves, so
+   Spark never guesses catalog names.
+3. **User configuration always wins**: A catalog the user configured by hand 
is never touched, and
+   this is enforced by mechanism rather than by convention.
+4. **Zero impact when disabled**: Users who do not add the new plugin see no 
behavior change.
+
+---
+
+## Non-Goals
+
+1. **Engines beyond Spark**: Flink and Trino may reuse the listing endpoint 
later.
+2. **Iceberg REST specification changes**: The listing endpoint is a 
Gravitino-private extension.
+
+---
+
+## Proposal
+
+Two new pieces — a **catalog-listing endpoint** on the Iceberg REST server and 
a single Spark
+plugin, `GravitinoLakehouseRESTDiscoveryPlugin`, that consumes it — plus one 
**ordering rule** that
+makes the interaction with `GravitinoSparkPlugin` deterministic. Each 
lakehouse format plugs into
+the plugin as a provider; V1 ships Iceberg.
+
+### Catalog-listing endpoint
+
+#### GET `{iceberg-rest-base}/gravitino/v1/catalogs`
+
+Placed outside the Iceberg REST specification's `/v1/` namespace (default 
deployment:
+`http://<host>:9001/iceberg/gravitino/v1/catalogs`) to mark it as a 
Gravitino-private extension.
+
+**Request:** No parameters.
+
+**Response:** `200 OK`
+
+```json
+{
+  "catalogs": [
+    { "name": "iceberg_prod" },
+    { "name": "iceberg_audit" }
+  ]
+}
+```
+
+| Field | Type | Description |
+|-------|------|-------------|
+| `catalogs[].name` | string | Catalog name as accepted by this server's 
`warehouse` parameter |
+
+The response only enumerates names: per-catalog client configuration already 
arrives via
+`GET /v1/config?warehouse=<name>`. Because JSON objects extend compatibly, 
per-catalog fields can be
+added later if an engine ever needs registration-time metadata (e.g. Trino's 
vending flag), so V1
+does not speculatively define any.
+
+### GravitinoLakehouseRESTDiscoveryPlugin
+
+Once configured, this single plugin fetches the catalog list from each 
configured REST server at
+Spark session startup and writes the corresponding `spark.sql.catalog.*` 
entries, so users no longer
+hand-write them. Which catalogs are registered, and under what Spark name, is 
decided by
+`CatalogRegistrationPolicy`.
+
+```text
+spark.plugins=org.apache.gravitino.spark.connector.plugin.GravitinoLakehouseRESTDiscoveryPlugin
+```
+
+The plugin is format-agnostic. Each lakehouse format is a **provider** — 
Iceberg in V1, Lance later
+(see [Lance support](#lance-support)) — carrying its own engine runtime and 
config prefix
+(`icebergRest.*`, `lanceRest.*`). A provider is active only when its `uri` is 
set, so the URI
+doubles as the per-format switch and no `enable*` flag is needed; if a `uri` 
is set but its provider
+is not on the classpath, the plugin fails fast. Dependency isolation is 
preserved — a user who needs
+only Iceberg puts only the Iceberg provider on the classpath — while 
`spark.plugins` lists one
+plugin and there is a single ordering rule.
+
+The registration policy and user-configuration precedence below are shared by 
every provider; the
+listing client, generated entries, and credential handling are 
provider-specific.
+
+#### Registration policy interface
+
+A single plugin-level policy, selected with 
`spark.sql.gravitino.rest.registrationPolicy`, applies
+to every provider (the default implementation applies when unset). Each method 
receives the `format`
+that advertised the catalog — the same token as the config prefix, `"iceberg"` 
or `"lance"` — so one
+policy can still apply format-specific rules without a policy per format.
+
+```java
+/** Decides whether an advertised REST catalog is registered, and under what 
Spark name. */
+@DeveloperApi
+public interface CatalogRegistrationPolicy {
+
+  /**
+   * Whether to register this catalog automatically as a Spark REST catalog.
+   *
+   * @param format the lakehouse format that advertised the catalog, e.g. 
"iceberg" or "lance"
+   * @param catalogName a catalog name advertised by that format's REST 
server; names already
+   *     claimed by user configuration are filtered out by the plugin and 
never reach this method
+   * @return true to register, false to skip
+   */
+  boolean shouldRegister(String format, String catalogName);
+
+  /**
+   * The Spark catalog name to register an accepted catalog under. Defaults to 
the advertised name.
+   *
+   * @param format the lakehouse format that advertised the catalog
+   * @param catalogName the accepted catalog name
+   * @return the Spark catalog name
+   */
+  default String sparkCatalogName(String format, String catalogName) {
+    return catalogName;
+  }
+}
+```
+
+The default implementation registers every advertised catalog under its 
advertised name, keeping a
+1:1 identity between the Spark catalog name and the REST server catalog name. 
Deployments that need
+to register a subset, rename catalogs on the Spark side, or treat formats 
differently implement the
+interface and point `registrationPolicy` at their class.
+
+#### User configuration precedence
+
+User configuration has the highest priority; the plugin fills in only what the 
user left unset.
+Precedence for each registered catalog, high to low, enforced by the plugin so 
no policy can weaken
+it:
+
+1. **User implementation key** `spark.sql.catalog.<name>`: the user owns that 
name entirely — the
+   catalog is dropped before the policy runs and nothing is generated for it.
+2. **User per-catalog sub-key** (`spark.sql.catalog.<name>.<key>`): wins over 
the generated value,
+   including the provider's core routing keys.
+3. **Plugin-generated keys**: the implementation class and the provider's core 
routing keys.
+4. **Global `catalogProperties.<key>`**: copied in as a per-catalog default, 
so it never overrides a
+   generated key — a stray `catalogProperties` core key cannot hijack routing.
+
+Startup still fails fast only for policy output that cannot be resolved: a 
returned Spark name that
+duplicates another catalog's name, collides with a name the user already 
configured, or is not a
+valid Spark identifier.
+
+#### Iceberg provider
+
+The provider issues the listing request with Iceberg's own `RESTClient`
+(`org.apache.iceberg.rest.HTTPClient`, the one `RESTCatalog` uses), not the 
Gravitino client: the
+target is the Iceberg REST server, the Iceberg runtime is already on its 
classpath, and it parses
+`ErrorResponse` the same way table calls do (authentication below). It uses the
+`spark.sql.gravitino.icebergRest.*` prefix, disjoint from the existing 
plugin's keys:
+
+| Configuration | Required | Default | Description |
+|---------------|----------|---------|-------------|
+| `spark.sql.gravitino.icebergRest.uri` | Yes | None | Base URI of the Iceberg 
REST server, e.g. `http://127.0.0.1:9001/iceberg/`; setting it activates the 
Iceberg provider |

Review Comment:
   `icebergRest` -> `icebergREST`?



-- 
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]

Reply via email to