yifan-c commented on code in PR #4465:
URL: https://github.com/apache/cassandra/pull/4465#discussion_r2519942373
##########
test/unit/org/apache/cassandra/config/ConfigCompatibilityTest.java:
##########
@@ -225,7 +229,18 @@ private void diff(Loader loader, Map<Class<?>, Map<String,
Replacement>> replace
// previous is leaf, is current?
Map<String, Property> children =
Properties.isPrimitive(prop) || Properties.isCollection(prop) ?
Collections.emptyMap() : loader.getProperties(prop.getType());
if (!children.isEmpty())
+ {
errors.add(String.format("Property %s used to be a
value-type, but now is nested type %s", name, prop.getType()));
+
+ // Verify SettingsTable maps old name to new nested
path for backwards compatibility (e.g., "authenticator" ->
"authenticator.class_name")
+ if (!backwardsCompatNames.containsKey(name)) {
Review Comment:
this one too
##########
src/java/org/apache/cassandra/db/virtual/SettingsTable.java:
##########
@@ -163,8 +163,7 @@ private static Map<String, Property> getProperties()
assert conflict == null || r.oldName.equals(r.newName) :
String.format("New property %s attempted to replace %s, but this property
already exists", latest.getName(), conflict.getName());
}
}
- for (Map.Entry<String, String> e :
BACKWARDS_COMPATABLE_NAMES.entrySet())
- {
+ for (Map.Entry<String, String> e :
BACKWARDS_COMPATIBLE_NAMES.entrySet()) {
Review Comment:
place `{` on new line
##########
src/java/org/apache/cassandra/db/virtual/SettingsTable.java:
##########
@@ -183,15 +182,20 @@ private static Map<String, Property> getProperties()
* There were a handle full of properties which had custom names, names
not present in the yaml, this map also
* fixes this and returns the proper (what is accessable via yaml) names.
*/
- private static Map<String, String> getBackwardsCompatableNames()
- {
+ private static Map<String, String> getBackwardsCompatibleNames() {
Review Comment:
`{` placement
##########
src/java/org/apache/cassandra/db/virtual/SettingsTable.java:
##########
@@ -42,12 +42,12 @@
import org.apache.cassandra.service.ClientWarn;
import org.yaml.snakeyaml.introspector.Property;
-final class SettingsTable extends AbstractVirtualTable
+public final class SettingsTable extends AbstractVirtualTable
Review Comment:
It is change necessary? Please revert if not required by this patch.
--
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]
