Centralizes definitions of accumulo tables #4163
Conversation
|
|
||
| private static final Set<TableId> META_TABLES = | ||
| Set.of(RootTable.ID, MetadataTable.ID, FateTable.ID); | ||
| private static final Set<TableId> META_TABLES = Arrays.stream(AccumuloTables.values()) |
There was a problem hiding this comment.
This bit of code was the motivation for this PR.
|
Would it help merging forward if the AccumuloTables enum and just those related changes were targeted for 3.1? The changes that are not directly retreaded to elasticity seem to be general enough and an improvement that maybe they could be in 3.1? |
|
I like the consolidation! I also agree with @EdColeman that this change seems simple enough to target for main and hopefully reduce the merge conflicts for elasticity. Then the only elasticity specific change is the creation of a new |
|
@EdColeman and @ddanielr for making the change in main, thinking of the following strategy.
If that strategy sounds good I can do that w/o any further PRs. |
I think that's an easier strategy and keeps the elasticity complexity fixes from being hidden in a merge commit. |
|
Looks like elasticity is not currently up to do date with main. Working on that merge first. |
This was an automated change made by an IDE to inline constants that defined accumulo table ids and names.
This commit applies the same automated transformation to the elasticity branch that was applied to main in da7866e
keith-turner
force-pushed
the
accumulo-tables-enum
branch
from
db2a7d7 to
e74f64b
Compare
|
Made multiple commits to add the new AccumuloTable enum to main and elasticity. |
|
Just catching up on this now, this LGTM and makes a lot of sense, thanks for refactoring this. |
5 participants
Three tables in the "accumulo." namespace were defined as constants in the three classes in the code. The change gathers the definitions of these three tables into a single enum in the code. The motivation for this change was that the new fate code introduced in #4133 needed to know of the all the accumulo tables ids. The way the code was structured if a new accumulo.X table is added in the future someone may not realize the fate code needs to know about it. This change makes the dependency more apparent in the code.
Existing constants for the metadata and root table name and id were not inlined in the commit for two reasons. First, it would make this commit hard to review. Second, it would make merging main into the elasticity branch more difficult (its already tricky). Inlining these constants can easily be done as a stand alone commit later when the elasticity branch is ready to merge into main.