Skip to content
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

[#436] fix(catalog-lakehouse): create dummy reportMetrics interface to remove warning message when spark writing data using IcebergRestCatalog #449

Merged
merged 1 commit into from
Sep 26, 2023
+47 −0
Merged

[#436] fix(catalog-lakehouse): create dummy reportMetrics interface to remove warning message when spark writing data using IcebergRestCatalog #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions ...va/com/datastrato/graviton/catalog/lakehouse/iceberg/web/rest/IcebergTableOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.rest.RESTUtil;
import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.ReportMetricsRequest;
import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -127,4 +128,14 @@ public Response tableExists(
return IcebergRestUtils.notExists();
}
}

@POST
@Path("{table}/metrics")
@Produces(MediaType.APPLICATION_JSON)
public Response reportTableMetrics(
@PathParam("namespace") String namespace,
@PathParam("table") String table,
ReportMetricsRequest request) {
return IcebergRestUtils.noContent();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a plan to implement it in the future, and should TODO be added?

Copy link
Contributor

Choose a reason for hiding this comment

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

@sandflee please reply this comment, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes， this PR just add a mock interface to remove the spark warning message， How to store and retrieve the metrics may be an attractive feature， needs a detailed design。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add #453 to track

}
}
1 change: 1 addition & 0 deletions .../java/com/datastrato/graviton/catalog/lakehouse/iceberg/web/rest/IcebergRestTestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class IcebergRestTestUtil {
public static final String TEST_NAMESPACE_NAME = "graviton-test";
public static final String TABLE_PATH = NAMESPACE_PATH + "/" + TEST_NAMESPACE_NAME + "/tables";
public static final String RENAME_TABLE_PATH = V_1 + "/tables/rename";
public static final String REPORT_METRICS_POSTFIX = "metrics";

public static final boolean DEBUG_SERVER_LOG_ENABLED = true;

Expand Down
8 changes: 8 additions & 0 deletions ...test/java/com/datastrato/graviton/catalog/lakehouse/iceberg/web/rest/IcebergTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ public Builder getTableClientBuilder(Optional<String> name) {
return getIcebergClientBuilder(path, Optional.empty());
}

public Builder getReportMetricsClientBuilder(String name) {
String path =
Joiner.on("/")
.skipNulls()
.join(IcebergRestTestUtil.TABLE_PATH, name, IcebergRestTestUtil.REPORT_METRICS_POSTFIX);
return getIcebergClientBuilder(path, Optional.empty());
}

public Builder getNamespaceClientBuilder() {
return getNamespaceClientBuilder(Optional.empty(), Optional.empty());
}
Expand Down
27 changes: 27 additions & 0 deletions ...om/datastrato/graviton/catalog/lakehouse/iceberg/web/rest/TestIcebergTableOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@
import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.metrics.CommitReport;
import org.apache.iceberg.metrics.ImmutableCommitMetricsResult;
import org.apache.iceberg.metrics.ImmutableCommitReport;
import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.RenameTableRequest;
import org.apache.iceberg.rest.requests.ReportMetricsRequest;
import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.apache.iceberg.rest.responses.ListTablesResponse;
import org.apache.iceberg.rest.responses.LoadTableResponse;
Expand Down Expand Up @@ -284,4 +288,27 @@ void testRenameTable(boolean withPrefix) {
verifyCreateTableSucc("rename_foo3");
verifyRenameTableFail("rename_foo2", "rename_foo3", 409);
}

@Test
void testReportTableMetrics() {

verifyCreateNamespaceSucc(IcebergRestTestUtil.TEST_NAMESPACE_NAME);
verifyCreateTableSucc("metrics_foo1");

ImmutableCommitMetricsResult commitMetrics = ImmutableCommitMetricsResult.builder().build();
CommitReport commitReport =
ImmutableCommitReport.builder()
.tableName("metrics_foo1")
.snapshotId(-1)
.sequenceNumber(-1)
.operation("append")
.commitMetrics(commitMetrics)
.build();
ReportMetricsRequest request = ReportMetricsRequest.of(commitReport);
Response response =
getReportMetricsClientBuilder("metrics_foo1")
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));

Assertions.assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus());
}
}