Skip to content

[#9009]improvement(lance-rest-server): add ITs for LRS namespace operations #9008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jerryshao merged 8 commits into from
Nov 4, 2025
Merged

[#9009]improvement(lance-rest-server): add ITs for LRS namespace operations #9008

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
33d17b9
add ITs for LRS namespace operations
mchades Nov 3, 2025
e91be3f
fix IT
mchades Nov 3, 2025
65ec483
fix container dep
mchades Nov 3, 2025
c9a738d
fix deploy mode
mchades Nov 3, 2025
9b47908
fix dep
mchades Nov 3, 2025
657bb49
fix miss
mchades Nov 3, 2025
a289055
address comments
mchades Nov 4, 2025
0384cf5
address comments fron yuqi
mchades Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergBaseIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public abstract class CatalogIcebergBaseIT extends BaseIT {

@BeforeAll
public void startup() throws Exception {
super.ignoreAuxRestService = false;
super.ignoreIcebergAuxRestService = false;
super.startIntegrationTest();
containerSuite.startHiveContainer();
initIcebergCatalogProperties();
Expand Down
2 changes: 1 addition & 1 deletion ...he/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergKerberosHiveIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void startIntegrationTest() {
// Config kerberos configuration for Gravitino server
addKerberosConfig();

super.ignoreAuxRestService = false;
super.ignoreIcebergAuxRestService = false;
// Start Gravitino server
super.startIntegrationTest();
} catch (Exception e) {
Expand Down
2 changes: 1 addition & 1 deletion ...flink/src/test/java/org/apache/gravitino/flink/connector/integration/test/FlinkEnvIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected String flinkByPass(String key) {
protected abstract String getProvider();

private void initIcebergRestServiceEnv() {
super.ignoreAuxRestService = false;
super.ignoreIcebergAuxRestService = false;
Map<String, String> icebergRestServiceConfigs = new HashMap<>();
icebergRestServiceConfigs.put(
"gravitino."
Expand Down
3 changes: 3 additions & 0 deletions integration-test-common/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ dependencies {
testImplementation(project(":clients:client-java"))
testImplementation(project(":common"))
testImplementation(project(":core"))
testImplementation(project(":lance:lance-common")) {
exclude("*")
}
testImplementation(project(":server"))
testImplementation(project(":server-common"))
testImplementation(libs.bundles.jetty)
Expand Down
74 changes: 62 additions & 12 deletions ...ration-test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitino.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
package org.apache.gravitino.integration.test;

import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
import static org.apache.gravitino.lance.common.config.LanceConfig.GRAVITINO_NAMESPACE_BACKEND;
import static org.apache.gravitino.lance.common.config.LanceConfig.LANCE_CONFIG_PREFIX;
import static org.apache.gravitino.lance.common.config.LanceConfig.METALAKE_NAME;
import static org.apache.gravitino.lance.common.config.LanceConfig.NAMESPACE_BACKEND;
import static org.apache.gravitino.lance.common.config.LanceConfig.NAMESPACE_BACKEND_URI;

import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
Expand All @@ -42,6 +47,7 @@
import org.apache.gravitino.auxiliary.AuxiliaryServiceManager;
import org.apache.gravitino.client.HTTPClient;
import org.apache.gravitino.client.RESTClient;
import org.apache.gravitino.integration.test.util.GravitinoITUtils;
import org.apache.gravitino.integration.test.util.HttpUtils;
import org.apache.gravitino.integration.test.util.ITUtils;
import org.apache.gravitino.integration.test.util.KerberosProviderHelper;
Expand Down Expand Up @@ -78,11 +84,25 @@ public MiniGravitino(MiniGravitinoContext context) throws IOException {
mockConfDir.mkdirs();
}

private void removeAuxRestConfiguration(Properties properties) {
// Disable Iceberg REST service
properties.remove(
AuxiliaryServiceManager.GRAVITINO_AUX_SERVICE_PREFIX
+ AuxiliaryServiceManager.AUX_SERVICE_NAMES);
private void removeAuxRestConfiguration(Properties properties, String serviceToRemove) {
String value =
properties.getProperty(
AuxiliaryServiceManager.GRAVITINO_AUX_SERVICE_PREFIX
+ AuxiliaryServiceManager.AUX_SERVICE_NAMES);
if (value != null && !value.isEmpty()) {
List<String> serviceNames = COMMA.splitToList(value);

@jerryshao jerryshao Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this splitToList also do the trim to avoid some invisible characters like white space?

@mchades mchades Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. Here is the definition of COMMA:

private static final Splitter COMMA = Splitter.on(",").omitEmptyStrings().trimResults();

List<String> updatedServiceNames = new ArrayList<>();
for (String serviceName : serviceNames) {
if (!serviceName.equalsIgnoreCase(serviceToRemove)) {
updatedServiceNames.add(serviceName);
}
}
String updatedValue = String.join(",", updatedServiceNames);

@jerryshao jerryshao Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the updatedServiceNames is empty, shall we remove the config?

@mchades mchades Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

properties.setProperty(
AuxiliaryServiceManager.GRAVITINO_AUX_SERVICE_PREFIX
+ AuxiliaryServiceManager.AUX_SERVICE_NAMES,
updatedValue);
}
}

public void start() throws Exception {
Expand All @@ -103,9 +123,18 @@ public void start() throws Exception {
serverConfig.loadPropertiesFromFile(
new File(ITUtils.joinPath(mockConfDir.getAbsolutePath(), "gravitino.conf")));

// Disable auxiliary rest service.
if (context.ignoreAuxRestService) {
removeAuxRestConfiguration(properties);
if (context.ignoreIcebergAuxRestService) {
// Disable auxiliary rest service.
removeAuxRestConfiguration(properties, "iceberg-rest");
LOG.info("Iceberg auxiliary REST service is disabled for MiniGravitino.");
ITUtils.overwriteConfigFile(
ITUtils.joinPath(mockConfDir.getAbsolutePath(), "gravitino.conf"), properties);
}

if (context.ignoreLanceAuxRestService) {
// Disable auxiliary rest service.
removeAuxRestConfiguration(properties, "lance-rest");
LOG.info("Lance auxiliary REST service is disabled for MiniGravitino.");
ITUtils.overwriteConfigFile(
ITUtils.joinPath(mockConfDir.getAbsolutePath(), "gravitino.conf"), properties);
}
Expand Down Expand Up @@ -230,19 +259,40 @@ Map<String, String> getIcebergRestServiceConfigs() throws IOException {
return customConfigs;
}

private Map<String, String> getLanceRestServiceConfigs() throws IOException {
private Map<String, String> getLanceRestServiceConfigs(Map<String, String> configMap)
throws IOException {
if (context.ignoreLanceAuxRestService) {
return new HashMap<>();
}

@jerryshao jerryshao Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Collections.emptyMap().


Map<String, String> customConfigs = new HashMap<>();

String lanceJarPath = Paths.get("lance", "lance-rest-server", "build", "libs").toString();
String lanceConfigPath =
Paths.get("lance", "lance-rest-server", "src", "main", "resources").toString();
customConfigs.put(
"gravitino.lance-rest." + AuxiliaryServiceManager.AUX_SERVICE_CLASSPATH,
LANCE_CONFIG_PREFIX + AuxiliaryServiceManager.AUX_SERVICE_CLASSPATH,
String.join(",", lanceJarPath, lanceConfigPath));

customConfigs.put(
"gravitino.lance-rest." + JettyServerConfig.WEBSERVER_HTTP_PORT.getKey(),
LANCE_CONFIG_PREFIX + JettyServerConfig.WEBSERVER_HTTP_PORT.getKey(),
String.valueOf(RESTUtils.findAvailablePort(4000, 5000)));

if (GRAVITINO_NAMESPACE_BACKEND.equals(
configMap.getOrDefault(NAMESPACE_BACKEND.getKey(), NAMESPACE_BACKEND.getDefaultValue()))) {
// Set the Lance REST service to use the Gravitino server URI
String gravitinoUri =
String.format(
"http://%s:%s",
"localhost",
configMap.get(
GravitinoServer.WEBSERVER_CONF_PREFIX
+ JettyServerConfig.WEBSERVER_HTTP_PORT.getKey()));
customConfigs.put(LANCE_CONFIG_PREFIX + NAMESPACE_BACKEND_URI.getKey(), gravitinoUri);

String metalakeName = GravitinoITUtils.genRandomName("LanceRESTService_metalake");
customConfigs.put(LANCE_CONFIG_PREFIX + METALAKE_NAME.getKey(), metalakeName);

@jerryshao jerryshao Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we create the metalake here before using Lance namespace server?

@mchades mchades Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought is that the test class can use getLanceRESTServerMetalakeName() in BaseIT to create the metalake before the test and perform cleanup afterward. If we create the metalake here, it will be difficult to clean up.

}
return customConfigs;
}

Expand All @@ -255,7 +305,7 @@ private void customizeConfigFile(String configTempFileName, String configFileNam
String.valueOf(RESTUtils.findAvailablePort(2000, 3000)));

configMap.putAll(getIcebergRestServiceConfigs());
configMap.putAll(getLanceRestServiceConfigs());
configMap.putAll(getLanceRestServiceConfigs(configMap));
configMap.putAll(context.customConfig);

ITUtils.rewriteConfigFile(configTempFileName, configFileName, configMap);
Expand Down
11 changes: 8 additions & 3 deletions ...test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitinoContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@

public class MiniGravitinoContext {
Map<String, String> customConfig;
final boolean ignoreAuxRestService;
final boolean ignoreIcebergAuxRestService;
final boolean ignoreLanceAuxRestService;

public MiniGravitinoContext(Map<String, String> customConfig, boolean ignoreAuxRestService) {
public MiniGravitinoContext(
Map<String, String> customConfig,
boolean ignoreIcebergAuxRestService,
boolean ignoreLanceAuxRestService) {
this.customConfig = customConfig;
this.ignoreAuxRestService = ignoreAuxRestService;
this.ignoreIcebergAuxRestService = ignoreIcebergAuxRestService;
this.ignoreLanceAuxRestService = ignoreLanceAuxRestService;
}
}
25 changes: 23 additions & 2 deletions integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import static org.apache.gravitino.Configs.ENTITY_RELATIONAL_JDBC_BACKEND_PATH;
import static org.apache.gravitino.integration.test.util.TestDatabaseName.PG_CATALOG_POSTGRESQL_IT;
import static org.apache.gravitino.integration.test.util.TestDatabaseName.PG_JDBC_BACKEND;
import static org.apache.gravitino.lance.common.config.LanceConfig.LANCE_CONFIG_PREFIX;
import static org.apache.gravitino.lance.common.config.LanceConfig.METALAKE_NAME;
import static org.apache.gravitino.server.GravitinoServer.WEBSERVER_CONF_PREFIX;

import com.google.common.base.Splitter;
Expand Down Expand Up @@ -102,7 +104,9 @@ public class BaseIT {

protected Map<String, String> customConfigs = new HashMap<>();

protected boolean ignoreAuxRestService = true;
protected boolean ignoreIcebergAuxRestService = true;

protected boolean ignoreLanceAuxRestService = true;

public String DOWNLOAD_MYSQL_JDBC_DRIVER_URL =
"https://repo1.maven.org/maven2/mysql/mysql-connector-java/8.0.26/mysql-connector-java-8.0.26.jar";
Expand Down Expand Up @@ -133,6 +137,16 @@ public void registerCustomConfigs(Map<String, String> configs) {
customConfigs.putAll(configs);
}

protected int getLanceRESTServerPort() {
JettyServerConfig lanceServerConfig =
JettyServerConfig.fromConfig(serverConfig, LANCE_CONFIG_PREFIX);
return lanceServerConfig.getHttpPort();
}

protected String getLanceRESTServerMetalakeName() {
return serverConfig.getRawString(LANCE_CONFIG_PREFIX + METALAKE_NAME.getKey());
}

private void rewriteGravitinoServerConfig() throws IOException {
String gravitinoHome = System.getenv("GRAVITINO_HOME");
Path configPath = Paths.get(gravitinoHome, "conf", GravitinoServer.CONF_FILE);
Expand Down Expand Up @@ -330,11 +344,18 @@ public void startIntegrationTest() throws Exception {
serverConfig = new ServerConfig();
customConfigs.put(ENTITY_RELATIONAL_JDBC_BACKEND_PATH.getKey(), file.getAbsolutePath());
if (testMode != null && testMode.equals(ITUtils.EMBEDDED_TEST_MODE)) {
MiniGravitinoContext context = new MiniGravitinoContext(customConfigs, ignoreAuxRestService);
MiniGravitinoContext context =
new MiniGravitinoContext(
customConfigs, ignoreIcebergAuxRestService, ignoreLanceAuxRestService);
miniGravitino = new MiniGravitino(context);
miniGravitino.start();
serverConfig = miniGravitino.getServerConfig();
} else {
if (!ignoreLanceAuxRestService) {

@yuqi1129 yuqi1129 Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we move the code ignoreLanceAuxRestService and ignoreRestAuxRestService to L346 or before, I noticed that the same logic is there.

@mchades mchades Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

customConfigs.put(
LANCE_CONFIG_PREFIX + METALAKE_NAME.getKey(),
GravitinoITUtils.genRandomName("LanceRESTService_metalake"));
}
rewriteGravitinoServerConfig();
serverConfig.loadFromFile(GravitinoServer.CONF_FILE);

Expand Down
19 changes: 14 additions & 5 deletions .../java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.client.GravitinoClient;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NonEmptyCatalogException;
Expand Down Expand Up @@ -332,7 +333,8 @@ private CreateNamespaceResponse createOrUpdateCatalog(
// Catalog exists, handle based on mode
switch (mode) {
case EXIST_OK:
response.setProperties(Maps.newHashMap());
response.setProperties(
Optional.ofNullable(catalog.properties()).orElse(Collections.emptyMap()));
return response;
case CREATE:
throw LanceNamespaceException.conflict(
Expand All @@ -344,7 +346,7 @@ private CreateNamespaceResponse createOrUpdateCatalog(
CatalogChange[] changes =
buildChanges(
properties,
catalog.properties(),
removeInUseProperty(catalog.properties()),
CatalogChange::setProperty,
CatalogChange::removeProperty,
CatalogChange[]::new);
Expand All @@ -356,6 +358,12 @@ private CreateNamespaceResponse createOrUpdateCatalog(
}
}

private Map<String, String> removeInUseProperty(Map<String, String> properties) {
return properties.entrySet().stream()
.filter(e -> !e.getKey().equalsIgnoreCase(Catalog.PROPERTY_IN_USE))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}

@jerryshao jerryshao Nov 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we remove this?

@mchades mchades Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used to remove the in-use property from the old catalog when creating a catalog namespace with overwrite mode.

Because we use removeOldCatalogProperty and setNewCatalogProperty to achieve this, the in-use property in the catalog is reserved and cannot be used in removePropertyChange.

private CreateNamespaceResponse createOrUpdateSchema(
String catalogName,
String schemaName,
Expand All @@ -378,7 +386,8 @@ private CreateNamespaceResponse createOrUpdateSchema(
// Schema exists, handle based on mode
switch (mode) {
case EXIST_OK:
response.setProperties(Maps.newHashMap());
response.setProperties(
Optional.ofNullable(schema.properties()).orElse(Collections.emptyMap()));
return response;
case CREATE:
throw LanceNamespaceException.conflict(
Expand Down Expand Up @@ -422,9 +431,9 @@ private DropNamespaceResponse dropCatalog(
}
return new DropNamespaceResponse(); // SKIP mode
}
} catch (NonEmptyCatalogException e) {
} catch (NonEmptyCatalogException | CatalogInUseException e) {
throw LanceNamespaceException.badRequest(
String.format("Catalog %s is not empty.", catalogName),
String.format("Catalog %s is not empty or in used", catalogName),
NonEmptyCatalogException.class.getSimpleName(),
catalogName,
CommonUtil.formatCurrentStackTrace());
Expand Down
17 changes: 17 additions & 0 deletions lance/lance-rest-server/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,24 @@ dependencies {
implementation(libs.jackson.datatype.jdk8)
implementation(libs.jackson.datatype.jsr310)

testImplementation(project(":clients:client-java"))
testImplementation(project(":server"))
testImplementation(project(":integration-test-common", "testArtifacts"))

testImplementation(libs.commons.io)
testImplementation(libs.jersey.test.framework.core) {
exclude(group = "org.junit.jupiter")
}
testImplementation(libs.jersey.test.framework.provider.jetty) {
exclude(group = "org.junit.jupiter")
}

testImplementation(libs.junit.jupiter.api)
testImplementation(libs.mockito.inline)
testImplementation(libs.mysql.driver)
testImplementation(libs.postgresql.driver)
testImplementation(libs.testcontainers)

testRuntimeOnly(libs.junit.jupiter.engine)
}

Expand Down
Loading