This is an automated email from the ASF dual-hosted git repository.
timbrown pushed a commit to branch 297-properties-based-config-2
in repository https://gitbox.apache.org/repos/asf/incubator-xtable.git
The following commit(s) were added to refs/heads/297-properties-based-config-2
by this push:
new 4a27c5d9 move data path into base
4a27c5d9 is described below
commit 4a27c5d992e154c5e1f10f8f4b34c5b47a8ef888
Author: Timothy Brown <[email protected]>
AuthorDate: Sun Jul 7 20:04:17 2024 -0500
move data path into base
---
.../apache/xtable/conversion/ExternalTable.java | 3 ++
.../org/apache/xtable/conversion/SourceTable.java | 7 +---
.../org/apache/xtable/conversion/TargetTable.java | 3 +-
.../xtable/conversion/TestExternalTable.java | 29 ++++++++++---
.../apache/xtable/conversion/TestSourceTable.java | 47 ----------------------
.../apache/xtable/hudi/HudiConversionTarget.java | 2 +-
.../xtable/iceberg/IcebergConversionTarget.java | 10 ++---
.../org/apache/xtable/iceberg/TestIcebergSync.java | 1 -
8 files changed, 33 insertions(+), 69 deletions(-)
diff --git
a/xtable-api/src/main/java/org/apache/xtable/conversion/ExternalTable.java
b/xtable-api/src/main/java/org/apache/xtable/conversion/ExternalTable.java
index eeb690d2..b99dd75a 100644
--- a/xtable-api/src/main/java/org/apache/xtable/conversion/ExternalTable.java
+++ b/xtable-api/src/main/java/org/apache/xtable/conversion/ExternalTable.java
@@ -32,6 +32,7 @@ class ExternalTable {
@NonNull String name;
@NonNull String formatName;
@NonNull String metadataPath;
+ @NonNull String dataPath;
String[] namespace;
CatalogConfig catalogConfig;
@@ -39,11 +40,13 @@ class ExternalTable {
@NonNull String name,
@NonNull String formatName,
@NonNull String basePath,
+ String dataPath,
String[] namespace,
CatalogConfig catalogConfig) {
this.name = name;
this.formatName = formatName;
this.metadataPath = sanitizeBasePath(basePath);
+ this.dataPath = dataPath == null ? metadataPath :
sanitizeBasePath(dataPath);
this.namespace = namespace;
this.catalogConfig = catalogConfig;
}
diff --git
a/xtable-api/src/main/java/org/apache/xtable/conversion/SourceTable.java
b/xtable-api/src/main/java/org/apache/xtable/conversion/SourceTable.java
index b219cf12..b1a99771 100644
--- a/xtable-api/src/main/java/org/apache/xtable/conversion/SourceTable.java
+++ b/xtable-api/src/main/java/org/apache/xtable/conversion/SourceTable.java
@@ -20,13 +20,9 @@ package org.apache.xtable.conversion;
import lombok.Builder;
import lombok.EqualsAndHashCode;
-import lombok.Getter;
-import lombok.NonNull;
@EqualsAndHashCode(callSuper = true)
-@Getter
public class SourceTable extends ExternalTable {
- @NonNull String dataPath;
@Builder(toBuilder = true)
public SourceTable(
@@ -36,7 +32,6 @@ public class SourceTable extends ExternalTable {
String dataPath,
String[] namespace,
CatalogConfig catalogConfig) {
- super(name, formatName, metadataPath, namespace, catalogConfig);
- this.dataPath = dataPath == null ? this.metadataPath :
sanitizeBasePath(dataPath);
+ super(name, formatName, metadataPath, dataPath, namespace, catalogConfig);
}
}
diff --git
a/xtable-api/src/main/java/org/apache/xtable/conversion/TargetTable.java
b/xtable-api/src/main/java/org/apache/xtable/conversion/TargetTable.java
index 067eb8a9..276607a4 100644
--- a/xtable-api/src/main/java/org/apache/xtable/conversion/TargetTable.java
+++ b/xtable-api/src/main/java/org/apache/xtable/conversion/TargetTable.java
@@ -35,10 +35,11 @@ public class TargetTable extends ExternalTable {
String name,
String formatName,
String metadataPath,
+ String dataPath,
String[] namespace,
CatalogConfig catalogConfig,
Duration metadataRetention) {
- super(name, formatName, metadataPath, namespace, catalogConfig);
+ super(name, formatName, metadataPath, dataPath, namespace, catalogConfig);
this.metadataRetention =
metadataRetention == null ? Duration.of(7, ChronoUnit.DAYS) :
metadataRetention;
}
diff --git
a/xtable-api/src/test/java/org/apache/xtable/conversion/TestExternalTable.java
b/xtable-api/src/test/java/org/apache/xtable/conversion/TestExternalTable.java
index 6345baf0..21a8a14f 100644
---
a/xtable-api/src/test/java/org/apache/xtable/conversion/TestExternalTable.java
+++
b/xtable-api/src/test/java/org/apache/xtable/conversion/TestExternalTable.java
@@ -27,29 +27,46 @@ public class TestExternalTable {
@Test
void sanitizePath() {
ExternalTable tooManySlashes =
- new ExternalTable("name", "hudi", "s3://bucket//path", null, null);
+ new ExternalTable("name", "hudi", "s3://bucket//path", null, null,
null);
assertEquals("s3://bucket/path", tooManySlashes.getMetadataPath());
ExternalTable localFilePath =
- new ExternalTable("name", "hudi", "/local/data//path", null, null);
+ new ExternalTable("name", "hudi", "/local/data//path", null, null,
null);
assertEquals("file:///local/data/path", localFilePath.getMetadataPath());
ExternalTable properLocalFilePath =
- new ExternalTable("name", "hudi", "file:///local/data//path", null,
null);
+ new ExternalTable("name", "hudi", "file:///local/data//path", null,
null, null);
assertEquals("file:///local/data/path",
properLocalFilePath.getMetadataPath());
}
@Test
void errorIfRequiredArgsNotSet() {
assertThrows(
- NullPointerException.class, () -> new ExternalTable("name", "hudi",
null, null, null));
+ NullPointerException.class,
+ () -> new ExternalTable("name", "hudi", null, null, null, null));
assertThrows(
NullPointerException.class,
- () -> new ExternalTable("name", null, "file://bucket/path", null,
null));
+ () -> new ExternalTable("name", null, "file://bucket/path", null,
null, null));
assertThrows(
NullPointerException.class,
- () -> new ExternalTable(null, "hudi", "file://bucket/path", null,
null));
+ () -> new ExternalTable(null, "hudi", "file://bucket/path", null,
null, null));
+ }
+
+ @Test
+ void dataPathDefaultsToMetadataPath() {
+ String metadataPath = "file:///path/to/table";
+ ExternalTable externalTable = new ExternalTable("name", "hudi",
metadataPath, null, null, null);
+ assertEquals(metadataPath, externalTable.getDataPath());
+ }
+
+ @Test
+ void dataPathIsSanitized() {
+ String metadataPath = "file:///path/to/table";
+ String dataPath = "file:///path/to/table//data";
+ ExternalTable externalTable =
+ new ExternalTable("name", "hudi", metadataPath, dataPath, null, null);
+ assertEquals("file:///path/to/table/data", externalTable.getDataPath());
}
}
diff --git
a/xtable-api/src/test/java/org/apache/xtable/conversion/TestSourceTable.java
b/xtable-api/src/test/java/org/apache/xtable/conversion/TestSourceTable.java
deleted file mode 100644
index 42ea63da..00000000
--- a/xtable-api/src/test/java/org/apache/xtable/conversion/TestSourceTable.java
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * 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.
- */
-
-package org.apache.xtable.conversion;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-
-import org.junit.jupiter.api.Test;
-
-class TestSourceTable {
- @Test
- void dataPathDefaultsToMetadataPath() {
- String metadataPath = "file:///path/to/table";
- SourceTable sourceTable =
-
SourceTable.builder().name("name").formatName("hudi").metadataPath(metadataPath).build();
- assertEquals(metadataPath, sourceTable.getDataPath());
- }
-
- @Test
- void dataPathIsSanitized() {
- String metadataPath = "file:///path/to/table";
- String dataPath = "file:///path/to/table//data";
- SourceTable sourceTable =
- SourceTable.builder()
- .name("name")
- .formatName("hudi")
- .metadataPath(metadataPath)
- .dataPath(dataPath)
- .build();
- assertEquals("file:///path/to/table/data", sourceTable.getDataPath());
- }
-}
diff --git
a/xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionTarget.java
b/xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionTarget.java
index e95391ad..c5d4fae1 100644
--- a/xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionTarget.java
+++ b/xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionTarget.java
@@ -112,7 +112,7 @@ public class HudiConversionTarget implements
ConversionTarget {
Configuration configuration,
int maxNumDeltaCommitsBeforeCompaction) {
this(
- targetTable.getMetadataPath(),
+ targetTable.getDataPath(),
(int) targetTable.getMetadataRetention().toHours(),
maxNumDeltaCommitsBeforeCompaction,
BaseFileUpdatesExtractor.of(
diff --git
a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergConversionTarget.java
b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergConversionTarget.java
index fe9ddb37..abd2e74d 100644
---
a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergConversionTarget.java
+++
b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergConversionTarget.java
@@ -59,9 +59,9 @@ public class IcebergConversionTarget implements
ConversionTarget {
private IcebergDataFileUpdatesSync dataFileUpdatesExtractor;
private IcebergTableManager tableManager;
private String basePath;
+ private String dataPath;
private TableIdentifier tableIdentifier;
private IcebergCatalogConfig catalogConfig;
- private Configuration configuration;
private int snapshotRetentionInHours;
private Transaction transaction;
private Table table;
@@ -71,7 +71,6 @@ public class IcebergConversionTarget implements
ConversionTarget {
IcebergConversionTarget(
TargetTable targetTable,
- Configuration configuration,
IcebergSchemaExtractor schemaExtractor,
IcebergSchemaSync schemaSync,
IcebergPartitionSpecExtractor partitionSpecExtractor,
@@ -80,7 +79,6 @@ public class IcebergConversionTarget implements
ConversionTarget {
IcebergTableManager tableManager) {
_init(
targetTable,
- configuration,
schemaExtractor,
schemaSync,
partitionSpecExtractor,
@@ -91,7 +89,6 @@ public class IcebergConversionTarget implements
ConversionTarget {
private void _init(
TargetTable targetTable,
- Configuration configuration,
IcebergSchemaExtractor schemaExtractor,
IcebergSchemaSync schemaSync,
IcebergPartitionSpecExtractor partitionSpecExtractor,
@@ -105,7 +102,7 @@ public class IcebergConversionTarget implements
ConversionTarget {
this.dataFileUpdatesExtractor = dataFileUpdatesExtractor;
String tableName = targetTable.getName();
this.basePath = targetTable.getMetadataPath();
- this.configuration = configuration;
+ this.dataPath = targetTable.getDataPath();
this.snapshotRetentionInHours = (int)
targetTable.getMetadataRetention().toHours();
String[] namespace = targetTable.getNamespace();
this.tableIdentifier =
@@ -127,7 +124,6 @@ public class IcebergConversionTarget implements
ConversionTarget {
public void init(TargetTable targetTable, Configuration configuration) {
_init(
targetTable,
- configuration,
IcebergSchemaExtractor.getInstance(),
IcebergSchemaSync.getInstance(),
IcebergPartitionSpecExtractor.getInstance(),
@@ -178,7 +174,7 @@ public class IcebergConversionTarget implements
ConversionTarget {
updateProperties.set(TableSyncMetadata.XTABLE_METADATA, metadata.toJson());
if (!table.properties().containsKey(TableProperties.WRITE_DATA_LOCATION)) {
// Required for a consistent write location when writing back to the
table as Iceberg
- updateProperties.set(TableProperties.WRITE_DATA_LOCATION, basePath);
+ updateProperties.set(TableProperties.WRITE_DATA_LOCATION, dataPath);
}
if (!Boolean.parseBoolean(
table
diff --git
a/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSync.java
b/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSync.java
index d82c15ac..609e60ff 100644
--- a/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSync.java
+++ b/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSync.java
@@ -197,7 +197,6 @@ public class TestIcebergSync {
.metadataRetention(Duration.of(1, ChronoUnit.HOURS))
.formatName(TableFormat.ICEBERG)
.build(),
- CONFIGURATION,
mockSchemaExtractor,
mockSchemaSync,
mockPartitionSpecExtractor,
