liamzwbao commented on code in PR #1126:
URL: https://github.com/apache/polaris/pull/1126#discussion_r2011135426
##########
quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java:
##########
@@ -20,20 +20,37 @@
import io.quarkus.test.junit.QuarkusIntegrationTest;
import java.lang.reflect.Field;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import org.apache.iceberg.view.ViewCatalogTests;
import
org.apache.polaris.service.it.test.PolarisRestCatalogViewFileIntegrationTest;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.AnnotatedElementContext;
+import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.api.io.TempDirFactory;
@QuarkusIntegrationTest
public class QuarkusRestCatalogViewFileIT extends
PolarisRestCatalogViewFileIntegrationTest {
@BeforeEach
- public void setUpTempDir(@TempDir Path tempDir) throws Exception {
+ public void setUpTempDir(@TempDir(factory = CustomTempDirFactory.class) Path
tempDir)
+ throws Exception {
// see https://github.com/quarkusio/quarkus/issues/13261
Field field = ViewCatalogTests.class.getDeclaredField("tempDir");
field.setAccessible(true);
field.set(this, tempDir);
}
+
+ private static class CustomTempDirFactory implements TempDirFactory {
+ @Override
+ public Path createTempDirectory(
+ AnnotatedElementContext elementContext, ExtensionContext
extensionContext)
+ throws Exception {
+ Path basePath = Paths.get(BASE_LOCATION.replaceFirst("file://", ""));
Review Comment:
Thanks @gh-yzou!
The reason only this test failed is that `tempDir` in the other tests is
always used with the
[`withLocation`](https://github.com/apache/polaris/blob/4b4ab97c8aaf0dba6a42e3f5ef05a0699349b529/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java#L1175-L1177)
function. This function [prefixes the location generated by `tempDir` with
`defaultBaseLocation`](https://github.com/apache/polaris/blob/4b4ab97c8aaf0dba6a42e3f5ef05a0699349b529/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java#L865-L867),
which ensures the resulting path is always within `allowedLocations`.
However, in this test, we are setting a user-defined location for
`write.metadata.path`, and we do **not** (and should not) apply the same
transformation. As a result, the location falls outside of `allowedLocations`,
which causes the test to fail.
In short, I feel like this might be a bug, since #193 adds user-defined
locations to `allowedLocations`. We may just need to apply the same logic here
as well.
--
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]
