gaborkaszab commented on code in PR #14465:
URL: https://github.com/apache/iceberg/pull/14465#discussion_r2563812597
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3066,6 +3073,101 @@ public void testCommitStateUnknownNotReconciled() {
.satisfies(ex -> assertThat(((CommitStateUnknownException)
ex).getSuppressed()).isEmpty());
}
+ @Test
+ public void testCustomTableOperationsInjection() throws IOException {
+ AtomicBoolean customTableOpsCalled = new AtomicBoolean();
Review Comment:
Can't we use Mockito.spy to verify that the functions are called instead of
introducing booleans for this?
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3066,6 +3073,101 @@ public void testCommitStateUnknownNotReconciled() {
.satisfies(ex -> assertThat(((CommitStateUnknownException)
ex).getSuppressed()).isEmpty());
}
+ @Test
+ public void testCustomTableOperationsInjection() throws IOException {
+ AtomicBoolean customTableOpsCalled = new AtomicBoolean();
+ AtomicBoolean customTransactionTableOpsCalled = new AtomicBoolean();
+
+ // Custom RESTSessionCatalog that overrides table operations creation
+ class CustomRESTSessionCatalog extends RESTSessionCatalog {
+ CustomRESTSessionCatalog(
+ Function<Map<String, String>, RESTClient> clientBuilder,
+ BiFunction<SessionCatalog.SessionContext, Map<String, String>,
FileIO> ioBuilder) {
+ super(clientBuilder, ioBuilder);
+ }
+
+ @Override
+ protected RESTTableOperations newTableOps(
+ RESTClient restClient,
+ String path,
+ Supplier<Map<String, String>> headers,
+ FileIO fileIO,
+ TableMetadata current,
+ Set<Endpoint> supportedEndpoints) {
+ customTableOpsCalled.set(true);
+ return super.newTableOps(restClient, path, headers, fileIO, current,
supportedEndpoints);
Review Comment:
Somehow my comment got lost amongst other comments so let me repeat it here
separately:
I checked the test for this PR and apparently we test how to put a wrapper
around the ops creation by overriding RESTSessionCatalog and RESTCatalog
functionality, but we don't test the end-to-end use case. I think injecting a
custom table/view ops would be nice to see how this would work. Like injecting
an ops that adds extra header to requests and also verify that these headers
are present, since that was the original motivation of this PR.
--
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]
