keith-turner commented on code in PR #5692:
URL: https://github.com/apache/accumulo/pull/5692#discussion_r2177905199
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterControl.java:
##########
@@ -213,16 +215,20 @@ public synchronized void start(ServerType server,
Map<String,String> configOverr
Map<String,Integer> compactorGroups =
cluster.getConfig().getClusterServerConfiguration().getCompactorConfiguration();
for (Entry<String,Integer> e : compactorGroups.entrySet()) {
+ final String rg = e.getKey();
Review Comment:
Instead of silently changing the class, what about throwing an exception
instead? I feel like this will avoid mysterious behavior in the future when
writing new test. We would have to find all test that cause this be thrown now
and fix them, but after that is done if someone writes a new test that does
this when they run they will get a clear error.
```suggestion
final String rg = e.getKey();
if(rg.equals(Constants.DEFAULT_RESOURCE_GROUP_NAME) &&
!classToUse.equals(Compactor.class)){
throw new IllegalArgumentException("Must use
"+Compactor.class.getName()+" with resource group
"+DEFAULT_RESOURCE_GROUP_NAME);
}
```
--
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]
