ctubbsii commented on code in PR #5350:
URL: https://github.com/apache/accumulo/pull/5350#discussion_r1992475598
##########
server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooPropEditor.java:
##########
@@ -180,7 +179,8 @@ private void printProperties(final ServerContext context,
final PropStoreKey<?>
writer.printf(": Property scope: %s\n", scope);
writer.printf(": ZooKeeper path: %s\n", propKey.getPath());
writer.printf(": Name: %s\n", getDisplayName(propKey, context));
- writer.printf(": Id: %s\n", propKey.getId());
+ writer.printf(": Id: %s\n", propKey instanceof IdBasedPropStoreKey
+ ? ((IdBasedPropStoreKey<?>) propKey).getId() : "N/A");
Review Comment:
There is no such method. The "N/A" is specifically for the PropKey that is
*not* one that is id-based. So, there's no ID to print, and no canonical form
of the ID. It simply doesn't apply to PropKeys that aren't based on an ID.
The history of this is that the the PropKey was built around AbstractId.
While this makes sense for namespace and table prop keys, the system prop key
had a hack to base it on the InstanceId (which is an AbstractId). However, with
the removal of instance IDs from the PropKey entirely, it no longer makes sense
to have it based on the AbstractId. So, I helped @meatballspaghetti by
modifying this code to distinguish between PropKeys that are based on
AbstractId and those that aren't (like the system prop key), and made the
canonical representation of PropKey the associated path, rather than any
associated abstract ID (which does not always exist).
--
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]
