Mmuzaf commented on code in PR #2133:
URL: https://github.com/apache/cassandra/pull/2133#discussion_r1148129493


##########
src/java/org/apache/cassandra/db/virtual/SettingsTable.java:
##########
@@ -157,4 +174,85 @@ private static Map<String, String> 
getBackwardsCompatableNames()
 
         return names;
     }
+
+    /**
+     * This class is used to provide backwards compatable support for the 
settings table in case the {@link Config}
+     * metadata changes. This class will provide the old names for the 
properties, but will use the new name to
+     * get the value from the {@link Config} object.
+     * <p>
+     * Updating a configuration property object will throw an exception if you 
will try to update a deprecated property.
+     */
+    private static class BackwardsCompatableRegistry implements Registry
+    {
+        private final Registry registry;
+        private final Map<String, Replacement> replacements;
+        private final Set<String> uniquePropertyKeys;
+        public BackwardsCompatableRegistry(Registry registry)
+        {
+            this.registry = registry;
+            this.replacements = replacements(registry);
+            // Some configs kept the same name, but changed the type, so we 
need to make sure we don't return the same name twice.
+            this.uniquePropertyKeys = 
ImmutableSet.<String>builder().addAll(registry.keys()).addAll(replacements.keySet()).build();
+        }
+
+        @Override
+        public void set(String name, @Nullable Object value)
+        {
+            Replacement replacement = replacements.get(name);
+            if (replacement == null)
+                registry.set(name, value);
+            else
+                throw new ConfigurationException(String.format("Unable to set 
'%s' as it is deprecated and is read only; use '%s' instead", name, 
replacement.newName));

Review Comment:
   It can't break anything because the SettingsTable is not updatable now, so 
it's up to us in this patch to decide which set of properties to make updatable 
:-)
   
   AFAIU, the `Replacement` is used to support backward compatibility between 
the releases so that users can use their old configs with new releases smoothly 
keeping in mind that they have to migrate to a new version of the 
`cassandra.yaml` (e.g. export the real config from the cluster to the yaml 
file).
   
   So showing a replaced name in the SettingsTable might be OK, but if we make 
it updatable it might cause confusion - because both the replaced name and a 
new name are responsible for the same single thing,  and this does not lead the 
user to understand which is the _right_ property in the release he uses.
   
   If I'm missing something, just let me know and I'll fix it (it is easy).



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to