dimas-b commented on code in PR #2714:
URL: https://github.com/apache/polaris/pull/2714#discussion_r2395831720


##########
runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java:
##########
@@ -302,4 +302,36 @@ public ProductionReadinessCheck 
checkInsecureStorageSettings(
         ? ProductionReadinessCheck.OK
         : ProductionReadinessCheck.of(errors.toArray(new Error[0]));
   }
+
+  @Produces
+  public ProductionReadinessCheck checkOverlappingSiblingCheckSettings(
+      FeaturesConfiguration featureConfiguration) {
+    var optimizedSiblingCheck = FeatureConfiguration.OPTIMIZED_SIBLING_CHECK;
+    var errors = new ArrayList<Error>();
+    if 
(Boolean.parseBoolean(featureConfiguration.defaults().get(optimizedSiblingCheck.key())))
 {
+      errors.add(
+          Error.ofSevere(
+              "This setting is not recommended for production environments as 
it may lead to incorrect behavior, due to missing data for 
location_without_scheme column in case of migrating from older Polaris 
versions."

Review Comment:
   It looks like refining this check in upgrade situations requires some more 
thinking.
   
   I propose to keep the "severe" error, but add another dedicated option for 
users to acknowledge the risks involved in `OPTIMIZED_SIBLING_CHECK`... kind of 
similar to `ALLOW_INSECURE_STORAGE_TYPES`.
   
   I still believe this check belongs with the JDBC module since the risks come 
from there (EclipseLink is deprecated already).



-- 
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]

Reply via email to