pingtimeout commented on code in PR #1532:
URL: https://github.com/apache/polaris/pull/1532#discussion_r2081295980
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +233,38 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
ALLOW_SPECIFYING_FILE_IO_IMPL =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ALLOW_SPECIFYING_FILE_IO_IMPL")
+ .description(
+ "Config key for whether to allow setting the FILE_IO_IMPL using
catalog properties. "
+ + "Must only be enabled in dev/test environments, never in
production systems.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
Review Comment:
@eric-maynard You are correct in that this flag is redundant. However, I
believe the intent here is to make the consequences 100% explicit.
I can see how some newcomers may overlook the
`SUPPORTED_CATALOG_STORAGE_TYPES=[..., "FILE"]` during a configuration review
and not realize the consequences it could have. Whereas an environment
variable that contains "insecure" and "security risks" will definitely raise
eyebrows.
Think about deleting a Github repository where you have to go to the `Danger
Zone`, click "Delete, confirm, then confirm a second time, then type the
repository name and confirm a third time". Here, given the consequences, I
think it is worth having a double-confirmation of the `FILE` storage type.
--
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]
