ajantha-bhat commented on code in PR #5037:
URL: https://github.com/apache/iceberg/pull/5037#discussion_r913415909
##########
aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java:
##########
@@ -295,6 +299,30 @@ public void testDropNamespace() {
Assert.assertFalse("namespace must not exist", response.hasItem());
}
+ @Ignore
+ public void testRegisterTable() {
+ TableIdentifier identifier = TableIdentifier.of("a", "t1");
Review Comment:
probably gets a namespace doesn't exist error. Need to create a namespace
first? Also good to use `genRandomName()` as in other testcases.
##########
aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java:
##########
@@ -295,6 +299,30 @@ public void testDropNamespace() {
Assert.assertFalse("namespace must not exist", response.hasItem());
}
+ @Ignore
+ public void testRegisterTable() {
+ TableIdentifier identifier = TableIdentifier.of("a", "t1");
+ catalog.createTable(identifier, SCHEMA);
+ Table registeringTable = catalog.loadTable(identifier);
+ catalog.dropTable(identifier, false);
+ TableOperations ops = ((HasTableOperations) registeringTable).operations();
+ String metadataLocation = ((DynamoDbTableOperations)
ops).currentMetadataLocation();
+ Assertions.assertThat(catalog.registerTable(identifier,
metadataLocation)).isNotNull();
+ Table newTable = catalog.loadTable(identifier);
+ Assertions.assertThat(newTable).isNotNull();
+ }
+
+ @Ignore
+ public void testRegisterExistingTable() {
+ catalog.createTable(TableIdentifier.of("a", "t1"), SCHEMA);
Review Comment:
make a table identifier once and use in all 3 places.
and same comments as above.
##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java:
##########
@@ -433,4 +435,28 @@ public void testTablePropsDefinedAtCatalogLevel() {
"table-key5",
table.properties().get("key5"));
}
+
+ @Ignore
+ public void testRegisterTable() {
+ String namespace = createNamespace();
+ String tableName = getRandomName();
+ createTable(namespace, tableName);
+ Table table = glueCatalog.loadTable(TableIdentifier.of(namespace,
tableName));
+ String metadataLocation = ((BaseTable)
table).operations().current().metadataFileLocation();
+ glueCatalog.dropTable(TableIdentifier.of(namespace, tableName), false);
+ glueCatalog.registerTable(TableIdentifier.of(namespace, tableName),
metadataLocation);
Review Comment:
need to validate the results and test load table?
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java:
##########
@@ -589,4 +593,31 @@ public void testTablePropsDefinedAtCatalogLevel() throws
IOException {
"table-key5",
table.properties().get("key5"));
}
+
+ @Test
+ public void testRegisterTable() throws IOException {
+ TableIdentifier identifier = TableIdentifier.of("a", "t1");
+ TableIdentifier identifier2 = TableIdentifier.of("a", "t2");
+ HadoopCatalog catalog = hadoopCatalog();
+ catalog.createTable(identifier, SCHEMA);
+ Table registeringTable = catalog.loadTable(identifier);
+ TableOperations ops = ((HasTableOperations) registeringTable).operations();
+ String metadataLocation = ((HadoopTableOperations)
ops).current().metadataFileLocation();
+ Assertions.assertThat(catalog.registerTable(identifier2,
metadataLocation)).isNotNull();
+ Table newTable = catalog.loadTable(identifier2);
Review Comment:
nit: no need for the variable if it is used only once. Can be
`Assertions.assertThat(catalog.loadTable(identifier2)).isNotNull();`
##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java:
##########
@@ -433,4 +435,28 @@ public void testTablePropsDefinedAtCatalogLevel() {
"table-key5",
table.properties().get("key5"));
}
+
+ @Ignore
+ public void testRegisterTable() {
+ String namespace = createNamespace();
+ String tableName = getRandomName();
+ createTable(namespace, tableName);
+ Table table = glueCatalog.loadTable(TableIdentifier.of(namespace,
tableName));
+ String metadataLocation = ((BaseTable)
table).operations().current().metadataFileLocation();
+ glueCatalog.dropTable(TableIdentifier.of(namespace, tableName), false);
+ glueCatalog.registerTable(TableIdentifier.of(namespace, tableName),
metadataLocation);
+ }
+
+ @Ignore
+ public void testRegisterTableAlreadyExists() {
+ String namespace = createNamespace();
+ String tableName = getRandomName();
+ createTable(namespace, tableName);
+ Table table = glueCatalog.loadTable(TableIdentifier.of(namespace,
tableName));
+ String metadataLocation = ((BaseTable)
table).operations().current().metadataFileLocation();
+ AssertHelpers.assertThrows("Should fail to register to an existing Glue
table",
+ AlreadyExistsException.class,
+ "already exists in Glue",
Review Comment:
you many not get this error message `"already exists in Glue"`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
