ctubbsii commented on code in PR #5654:
URL: https://github.com/apache/accumulo/pull/5654#discussion_r2153080411
##########
server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader11to12Test.java:
##########
@@ -395,13 +401,20 @@ public void upgradeZooKeeperTest() throws Exception {
expect(zrw.exists("/problems")).andReturn(false).once();
- replay(context, zk, zrw);
+ expect(zrw.putPersistentData(isA(String.class), isA(byte[].class),
isA(NodeExistsPolicy.class)))
+ .andReturn(true).times(7);
+ expect(context.getPropStore()).andReturn(store).atLeastOnce();
+
expect(store.exists(isA(TablePropKey.class))).andReturn(false).atLeastOnce();
+ store.create(isA(TablePropKey.class), isA(Map.class));
+ expectLastCall().once();
Review Comment:
You could probably use anyObject() for these parameters. It would avoid the
need to suppress the warning, I think. Or, you could get more specific, and use
`eq()` to pass specific objects. For the Array, you could pass a more specific
matcher.
At the very least, a comment to explain where the count of 7 comes from
would be useful (3 for name/conf/status for 2 tables, plus 1 for the mapping
update? or something like that)
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java:
##########
@@ -406,6 +396,17 @@ private void
validateEmptyZKWorkerServerPaths(ServerContext context) {
}
}
+ private void initializeFateTable(ServerContext context) {
+ try {
+ Upgrader11to12.preparePre4_0NewTableState(context,
SystemTables.FATE.tableId(),
+ Namespace.ACCUMULO.id(), SystemTables.FATE.tableName(),
TableState.ONLINE,
+ ZooUtil.NodeExistsPolicy.FAIL);
+ } catch (InterruptedException | KeeperException ex) {
+ Thread.currentThread().interrupt();
Review Comment:
I don't think you need to re-set the interrupted flag here, because you're
exiting the current thread in the next line with the ISE. I'm also wondering if
this boilerplate exception handling can be pushed into the
`preparePre4_0NewTableState` method, so we can just inline this method. The
namespace, nodeexistspolicy, and online state can probably all be inlined.
--
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]
