ctubbsii commented on code in PR #5498:
URL: https://github.com/apache/accumulo/pull/5498#discussion_r2056700013
##########
core/src/main/java/org/apache/accumulo/core/metadata/AccumuloNamespace.java:
##########
@@ -28,7 +28,7 @@
/**
* Defines the name and id of all tables in the accumulo table namespace.
*/
-public enum AccumuloTable {
+public enum AccumuloNamespace {
Review Comment:
The rename makes sense for the .containsTable() method, but does not make
sense for the ROOT, METADATA, etc. constants. If they were named "ROOT_TABLE",
etc., then it might be more clear, but I agree it would be better if there was
a more concrete Table object type. I have long had an idea to create one for
the public API, with methods on it for scan, rename, delete, etc., and a
version internally, for tracking the resolved names and ids together for the
lifecycle of an operation. But, those won't necessarily solve this special case
of wanting to have hard-coded references to the built-in tables. Those
abstractions are better suited for the general use case.
These were previously on the `MetadataTable` class, but not all the builtin
tables are metadata tables, so an attempt was made in #4163 to consolidate them
into their own helper utility class.
Maybe a better name is AccumuloNamespaceTables (very clear, but long), or
BuiltinTables (shorter, but less clear)?
Perhaps it's better to pass on this change for now. I'm not sure there's a
perfect solution, and the current name is adequate, even if it is a little
confusing in some cases.
--
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]
