jerryshao commented on code in PR #3975:
URL: https://github.com/apache/gravitino/pull/3975#discussion_r1665084792
##########
.github/workflows/backend-integration-test.yml:
##########
@@ -61,7 +61,7 @@ jobs:
architecture: [linux/amd64]
java-version: [ 8, 11, 17 ]
test-mode: [ embedded, deploy ]
- backend: [ jdbcBackend, kvBackend]
+ backend: [ mysql, h2]
Review Comment:
I think maybe we can reduce the pipeline to combine embedded with h2, and
deploy with msyql, WDYT?
##########
catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java:
##########
@@ -118,28 +130,66 @@ public PropertiesMetadata topicPropertiesMetadata()
throws UnsupportedOperationE
@BeforeAll
public static void setUp() {
Config config = Mockito.mock(Config.class);
- Mockito.when(config.get(ENTITY_STORE)).thenReturn("kv");
-
Mockito.when(config.get(ENTITY_KV_STORE)).thenReturn(DEFAULT_ENTITY_KV_STORE);
- Mockito.when(config.get(Configs.ENTITY_SERDE)).thenReturn("proto");
-
Mockito.when(config.get(ENTITY_KV_ROCKSDB_BACKEND_PATH)).thenReturn(ROCKS_DB_STORE_PATH);
-
- Assertions.assertEquals(ROCKS_DB_STORE_PATH,
config.get(ENTITY_KV_ROCKSDB_BACKEND_PATH));
-
Mockito.when(config.get(STORE_TRANSACTION_MAX_SKEW_TIME)).thenReturn(1000L);
- Mockito.when(config.get(STORE_DELETE_AFTER_TIME)).thenReturn(20 * 60 *
1000L);
+ when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE);
+
when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE);
+
when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PATH)).thenReturn(STORE_PATH);
+
+ // The following properties are used to create the JDBC connection; they
are just for test, in
+ // the real world,
+ // they will be set automatically by the configuration file if you set
ENTITY_RELATIONAL_STORE
+ // as EMBEDDED_ENTITY_RELATIONAL_STORE.
+ when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL))
+ .thenReturn(String.format("jdbc:h2:%s;DB_CLOSE_DELAY=-1;MODE=MYSQL",
STORE_PATH));
Review Comment:
I found that you're hard coding to use H2, where do you use MySQL for test?
##########
gradle.properties:
##########
@@ -40,4 +40,6 @@ defaultScalaVersion = 2.12
pythonVersion = 3.8
# skipDockerTests is used to skip the tests that require Docker to be running.
-skipDockerTests = true
+skipDockerTests = false
Review Comment:
change back this one.
##########
core/src/test/java/com/datastrato/gravitino/storage/kv/TestEntityKeyEncoding.java:
##########
@@ -55,6 +56,7 @@
import org.mockito.Mockito;
@TestInstance(Lifecycle.PER_CLASS)
+@Disabled
Review Comment:
You should add the `Disabled` reason here and there.
##########
catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java:
##########
@@ -118,28 +130,66 @@ public PropertiesMetadata topicPropertiesMetadata()
throws UnsupportedOperationE
@BeforeAll
public static void setUp() {
Config config = Mockito.mock(Config.class);
- Mockito.when(config.get(ENTITY_STORE)).thenReturn("kv");
-
Mockito.when(config.get(ENTITY_KV_STORE)).thenReturn(DEFAULT_ENTITY_KV_STORE);
- Mockito.when(config.get(Configs.ENTITY_SERDE)).thenReturn("proto");
-
Mockito.when(config.get(ENTITY_KV_ROCKSDB_BACKEND_PATH)).thenReturn(ROCKS_DB_STORE_PATH);
-
- Assertions.assertEquals(ROCKS_DB_STORE_PATH,
config.get(ENTITY_KV_ROCKSDB_BACKEND_PATH));
-
Mockito.when(config.get(STORE_TRANSACTION_MAX_SKEW_TIME)).thenReturn(1000L);
- Mockito.when(config.get(STORE_DELETE_AFTER_TIME)).thenReturn(20 * 60 *
1000L);
+ when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE);
+
when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE);
+
when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PATH)).thenReturn(STORE_PATH);
+
+ // The following properties are used to create the JDBC connection; they
are just for test, in
+ // the real world,
+ // they will be set automatically by the configuration file if you set
ENTITY_RELATIONAL_STORE
+ // as EMBEDDED_ENTITY_RELATIONAL_STORE.
+ when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL))
+ .thenReturn(String.format("jdbc:h2:%s;DB_CLOSE_DELAY=-1;MODE=MYSQL",
STORE_PATH));
+
when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("gravitino");
+
when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("gravitino");
+
when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)).thenReturn("org.h2.Driver");
+
+ File f = FileUtils.getFile(STORE_PATH);
+ f.deleteOnExit();
+
+ when(config.get(VERSION_RETENTION_COUNT)).thenReturn(1L);
+ when(config.get(STORE_TRANSACTION_MAX_SKEW_TIME)).thenReturn(1000L);
+ when(config.get(STORE_DELETE_AFTER_TIME)).thenReturn(20 * 60 * 1000L);
+ when(config.get(ENTITY_SERDE)).thenReturn("proto");
store = EntityStoreFactory.createEntityStore(config);
store.initialize(config);
- store.setSerDe(EntitySerDeFactory.createEntitySerDe(config));
idGenerator = new RandomIdGenerator();
+
+ // Mock
+ MetalakeMetaService metalakeMetaService =
MetalakeMetaService.getInstance();
+ MetalakeMetaService spyMetaservice = Mockito.spy(metalakeMetaService);
+ doReturn(1L).when(spyMetaservice).getMetalakeIdByName(Mockito.anyString());
+
+ CatalogMetaService catalogMetaService = CatalogMetaService.getInstance();
+ CatalogMetaService spyCatalogMetaService = Mockito.spy(catalogMetaService);
+ doReturn(1L)
+ .when(spyCatalogMetaService)
+ .getCatalogIdByMetalakeIdAndName(Mockito.anyLong(),
Mockito.anyString());
+
+ MockedStatic<MetalakeMetaService> metalakeMetaServiceMockedStatic =
+ Mockito.mockStatic(MetalakeMetaService.class);
+ MockedStatic<CatalogMetaService> catalogMetaServiceMockedStatic =
+ Mockito.mockStatic(CatalogMetaService.class);
+
+ metalakeMetaServiceMockedStatic
+ .when(MetalakeMetaService::getInstance)
+ .thenReturn(spyMetaservice);
+ catalogMetaServiceMockedStatic
+ .when(CatalogMetaService::getInstance)
+ .thenReturn(spyCatalogMetaService);
}
@AfterAll
public static void tearDown() throws IOException {
store.close();
- FileUtils.deleteDirectory(FileUtils.getFile(ROCKS_DB_STORE_PATH));
new Path(TEST_ROOT_PATH)
.getFileSystem(new Configuration())
.delete(new Path(TEST_ROOT_PATH), true);
+
+ File f = FileUtils.getFile(H2_file);
+ System.gc();
Review Comment:
Why do you need this, can you explain more? It is strange to call gc
manually, do you have any other solution?
##########
core/src/main/java/com/datastrato/gravitino/EntityStoreFactory.java:
##########
@@ -54,6 +56,11 @@ public static EntityStore createEntityStore(Config config) {
String name = config.get(Configs.ENTITY_STORE);
String className = ENTITY_STORES.getOrDefault(name, name);
+ if (KV_STORE_KEY.equals(name)) {
+ throw new UnsupportedOperationException(
+ "KvEntityStore is not supported since this version. Please use
RelationalEntityStore instead.");
Review Comment:
Since which version?
--
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]
