Skip to content

Commit 94c94f1

Browse files
authored
Core: Remove configure: INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST (#1624)
1 parent f2d1caf commit 94c94f1

File tree

7 files changed

+52
-55
lines changed

7 files changed

+52
-55
lines changed

quarkus/defaults/src/main/resources/application.properties

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE"]
114114
polaris.features."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST"]
115115

116116
# realm overrides
117-
# polaris.features.realm-overrides."my-realm"."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"=true
118117
# polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true
119118

120119
# polaris.persistence.type=eclipse-link

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
100100
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
101101
import org.apache.polaris.core.persistence.pagination.PageToken;
102+
import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView;
102103
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
103104
import org.apache.polaris.core.secrets.UserSecretsManager;
104105
import org.apache.polaris.core.secrets.UserSecretsManagerFactory;
@@ -173,8 +174,6 @@ public Map<String, String> getConfigOverrides() {
173174
"true",
174175
"polaris.features.\"ALLOW_INSECURE_STORAGE_TYPES\"",
175176
"true",
176-
"polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
177-
"true",
178177
"polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
179178
"[\"FILE\",\"S3\"]",
180179
"polaris.features.\"LIST_PAGINATION_ENABLED\"",
@@ -227,6 +226,45 @@ public Map<String, String> getConfigOverrides() {
227226
private TestPolarisEventListener testPolarisEventListener;
228227
private ReservedProperties reservedProperties;
229228

229+
/**
230+
* A subclass of IcebergCatalog that adds FileIO management capabilities. This allows the file IO
231+
* logic to be encapsulated in a dedicated class.
232+
*/
233+
public static class IcebergFileIOCatalog extends IcebergCatalog {
234+
235+
public IcebergFileIOCatalog(
236+
PolarisEntityManager entityManager,
237+
PolarisMetaStoreManager metaStoreManager,
238+
CallContext callContext,
239+
PolarisResolutionManifestCatalogView resolvedEntityView,
240+
SecurityContext securityContext,
241+
TaskExecutor taskExecutor,
242+
FileIOFactory fileIOFactory,
243+
PolarisEventListener polarisEventListener) {
244+
super(
245+
entityManager,
246+
metaStoreManager,
247+
callContext,
248+
resolvedEntityView,
249+
securityContext,
250+
taskExecutor,
251+
fileIOFactory,
252+
polarisEventListener);
253+
}
254+
255+
@Override
256+
public synchronized FileIO getIo() {
257+
if (catalogFileIO == null) {
258+
catalogFileIO = loadFileIO(ioImplClassName, tableDefaultProperties);
259+
if (closeableGroup != null) {
260+
closeableGroup.addCloseable(catalogFileIO);
261+
}
262+
}
263+
264+
return catalogFileIO;
265+
}
266+
}
267+
230268
@BeforeAll
231269
public static void setUpMocks() {
232270
PolarisStorageIntegrationProviderImpl mock =
@@ -372,7 +410,7 @@ protected IcebergCatalog initCatalog(
372410
callContext, entityManager, securityContext, CATALOG_NAME);
373411
TaskExecutor taskExecutor = Mockito.mock();
374412
IcebergCatalog icebergCatalog =
375-
new IcebergCatalog(
413+
new IcebergFileIOCatalog(
376414
entityManager,
377415
metaStoreManager,
378416
callContext,
@@ -1004,7 +1042,7 @@ public void testUpdateNotificationCreateTableWithLocalFilePrefix() {
10041042
callContext, entityManager, securityContext, catalogWithoutStorage);
10051043
TaskExecutor taskExecutor = Mockito.mock();
10061044
IcebergCatalog catalog =
1007-
new IcebergCatalog(
1045+
new IcebergFileIOCatalog(
10081046
entityManager,
10091047
metaStoreManager,
10101048
callContext,
@@ -1071,7 +1109,7 @@ public void testUpdateNotificationCreateTableWithHttpPrefix() {
10711109
callContext, entityManager, securityContext, catalogName);
10721110
TaskExecutor taskExecutor = Mockito.mock();
10731111
IcebergCatalog catalog =
1074-
new IcebergCatalog(
1112+
new IcebergFileIOCatalog(
10751113
entityManager,
10761114
metaStoreManager,
10771115
callContext,

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,6 @@ public Map<String, String> getConfigOverrides() {
108108
"true",
109109
"polaris.features.\"ALLOW_INSECURE_STORAGE_TYPES\"",
110110
"true",
111-
"polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
112-
"true",
113111
"polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
114112
"[\"FILE\",\"S3\"]",
115113
"polaris.event-listener.type",

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,6 @@ public Map<String, String> getConfigOverrides() {
110110
"true",
111111
"polaris.features.\"ALLOW_INSECURE_STORAGE_TYPES\"",
112112
"true",
113-
"polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
114-
"true",
115113
"polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
116114
"[\"FILE\"]",
117115
"polaris.readiness.ignore-severe-issues",

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ public Map<String, String> getConfigOverrides() {
123123
return Map.of(
124124
"polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"",
125125
"true",
126-
"polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
127-
"true",
128126
"polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
129127
"[\"FILE\"]",
130128
"polaris.features.\"ALLOW_INSECURE_STORAGE_TYPES\"",

service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@
9191
import org.apache.polaris.core.catalog.PolarisCatalogHelpers;
9292
import org.apache.polaris.core.config.BehaviorChangeConfiguration;
9393
import org.apache.polaris.core.config.FeatureConfiguration;
94-
import org.apache.polaris.core.config.PolarisConfiguration;
9594
import org.apache.polaris.core.context.CallContext;
9695
import org.apache.polaris.core.entity.CatalogEntity;
9796
import org.apache.polaris.core.entity.NamespaceEntity;
@@ -170,14 +169,15 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
170169
private final SecurityContext securityContext;
171170
private final PolarisEventListener polarisEventListener;
172171

173-
private String ioImplClassName;
174-
private FileIO catalogFileIO;
172+
protected String ioImplClassName;
173+
protected FileIO catalogFileIO;
174+
protected CloseableGroup closeableGroup;
175+
protected Map<String, String> tableDefaultProperties;
176+
175177
private final String catalogName;
176178
private long catalogId = -1;
177179
private String defaultBaseLocation;
178-
private CloseableGroup closeableGroup;
179180
private Map<String, String> catalogProperties;
180-
private Map<String, String> tableDefaultProperties;
181181
private FileIOFactory fileIOFactory;
182182
private PolarisMetaStoreManager metaStoreManager;
183183

@@ -263,33 +263,13 @@ public void initialize(String name, Map<String, String> properties) {
263263
tableDefaultProperties =
264264
PropertyUtil.propertiesWithPrefix(properties, CatalogProperties.TABLE_DEFAULT_PREFIX);
265265

266-
if (initializeDefaultCatalogFileioForTest()) {
267-
LOGGER.debug(
268-
"Initializing a default catalogFileIO with properties {}", tableDefaultProperties);
269-
this.catalogFileIO = loadFileIO(ioImplClassName, tableDefaultProperties);
270-
closeableGroup.addCloseable(this.catalogFileIO);
271-
} else {
272-
LOGGER.debug("Not initializing default catalogFileIO");
273-
this.catalogFileIO = null;
274-
}
275-
276266
callContext.closeables().addCloseable(this);
277267
this.closeableGroup = new CloseableGroup();
278268
closeableGroup.addCloseable(metricsReporter());
279269
closeableGroup.setSuppressCloseFailure(true);
280270

281271
tableDefaultProperties =
282272
PropertyUtil.propertiesWithPrefix(properties, CatalogProperties.TABLE_DEFAULT_PREFIX);
283-
284-
if (initializeDefaultCatalogFileioForTest()) {
285-
LOGGER.debug(
286-
"Initializing a default catalogFileIO with properties {}", tableDefaultProperties);
287-
this.catalogFileIO = loadFileIO(ioImplClassName, tableDefaultProperties);
288-
closeableGroup.addCloseable(this.catalogFileIO);
289-
} else {
290-
LOGGER.debug("Not initializing default catalogFileIO");
291-
this.catalogFileIO = null;
292-
}
293273
}
294274

295275
public void setMetaStoreManager(PolarisMetaStoreManager newMetaStoreManager) {
@@ -359,8 +339,7 @@ public ViewBuilder buildView(TableIdentifier identifier) {
359339
@VisibleForTesting
360340
public TableOperations newTableOps(
361341
TableIdentifier tableIdentifier, boolean makeMetadataCurrentOnCommit) {
362-
return new BasePolarisTableOperations(
363-
catalogFileIO, tableIdentifier, makeMetadataCurrentOnCommit);
342+
return new BasePolarisTableOperations(getIo(), tableIdentifier, makeMetadataCurrentOnCommit);
364343
}
365344

366345
@Override
@@ -862,9 +841,10 @@ private Page<TableIdentifier> listViews(Namespace namespace, PageToken pageToken
862841
return listTableLike(PolarisEntitySubType.ICEBERG_VIEW, namespace, pageToken);
863842
}
864843

844+
@VisibleForTesting
865845
@Override
866846
protected ViewOperations newViewOps(TableIdentifier identifier) {
867-
return new BasePolarisViewOperations(catalogFileIO, identifier);
847+
return new BasePolarisViewOperations(getIo(), identifier);
868848
}
869849

870850
@Override
@@ -2514,7 +2494,7 @@ private Page<TableIdentifier> listTableLike(
25142494
* @param properties used to initialize the FileIO implementation
25152495
* @return FileIO object
25162496
*/
2517-
private FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
2497+
protected FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
25182498
IcebergTableLikeEntity icebergTableLikeEntity = IcebergTableLikeEntity.of(catalogEntity);
25192499
TableIdentifier identifier = icebergTableLikeEntity.getTableIdentifier();
25202500
Set<String> locations = Set.of(catalogEntity.getDefaultBaseLocation());
@@ -2545,12 +2525,6 @@ private Boolean getBooleanContextConfiguration(String configKey, boolean default
25452525
.getConfiguration(callContext.getPolarisCallContext(), configKey, defaultValue);
25462526
}
25472527

2548-
private boolean initializeDefaultCatalogFileioForTest() {
2549-
var ctx = callContext.getPolarisCallContext();
2550-
return ctx.getConfigurationStore()
2551-
.getConfiguration(ctx, INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST);
2552-
}
2553-
25542528
private int getMaxMetadataRefreshRetries() {
25552529
var ctx = callContext.getPolarisCallContext();
25562530
return ctx.getConfigurationStore()
@@ -2574,11 +2548,4 @@ private PageToken buildPageToken(@Nullable String tokenString, @Nullable Integer
25742548
return PageToken.build(tokenString, pageSize);
25752549
}
25762550
}
2577-
2578-
static final FeatureConfiguration<Boolean> INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST =
2579-
PolarisConfiguration.<Boolean>builder()
2580-
.key("INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST")
2581-
.defaultValue(false)
2582-
.description("")
2583-
.buildFeatureConfiguration();
25842551
}

site/content/in-dev/unreleased/configuration.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ read-only mode, as Polaris only reads the configuration file once, at startup.
9090
| `polaris.realm-context.header-name` | `Polaris-Realm` | Define the header name defining the realm context. |
9191
| `polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"` | `false` | Flag to enforce check if credential rotation. |
9292
| `polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"` | `FILE` | Define the catalog supported storage. Supported values are `S3`, `GCS`, `AZURE`, `FILE`. |
93-
| `polaris.features.realm-overrides."my-realm"."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"` | `true` | "Override" realm features, here the catalog init default flag. |
9493
| `polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"` | `true` | "Override" realm features, here the skip credential subscoping indirection flag. |
9594
| `polaris.authentication.authenticator.type` | `default` | Define the Polaris authenticator type. |
9695
| `polaris.authentication.token-service.type` | `default` | Define the Polaris token service type. |

0 commit comments

Comments
 (0)