[GitHub] [cassandra] adelapena commented on a diff in pull request #2027: CASSANDRA-18042 second approach DO NOT COMMIT

[GitBox <gi...@apache.org>]

adelapena commented on code in PR #2027:
URL: https://github.com/apache/cassandra/pull/2027#discussion_r1030333839


##########
src/java/org/apache/cassandra/db/guardrails/EnableFlag.java:
##########
@@ -93,7 +145,52 @@ public void ensureEnabled(@Nullable ClientState state)
      */
     public void ensureEnabled(String featureName, @Nullable ClientState state)
     {
-        if (!isEnabled(state))
-            fail(featureName + " is not allowed", state);
+        ensureInternal(featureName, state, () -> {
+            if (!enabled.test(state))
+            {
+                fail(featureName + " is not allowed", state);
+                return true;
+            }
+            return false;
+        });
+    }
+
+    /**
+     * Aborts the operation if this guardrail is enabled.
+     *
+     * <p>This must be called when the feature guarded by this guardrail is used to ensure such use is in fact
+     * disallowed.
+     *
+     * @param featureName The feature that is guarded by this guardrail (for reporting in error messages).
+     * @param state       The client state, used to skip the check if the query is internal or is done by a superuser. A
+     *                    {@code null} value means that the check should be done regardless of the query, although it
+     *                    won't throw any exception if the failure threshold is exceeded. This is so because checks
+     *                    without an associated client come from asynchronous processes such as compaction, and we don't
+     *                    want to interrupt such processes.
+     */
+    public void ensureDisabled(String featureName, @Nullable ClientState state)
+    {
+        ensureInternal(featureName, state, () -> {
+            if (enabled.test(state))
+            {
+                fail(featureName + " is allowed", state);

Review Comment:
   This is printing `0 default_time_to_live on a table with TimeWindowCompactionStrategy compaction strategy is allowed`, shouldn't it use the same `is not allowed` as `EnsureEnabled`?



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org