This is an automated email from the ASF dual-hosted git repository. ctubbsii pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new 5aaeb011f3 Address TODOs found in code (#2648) 5aaeb011f3 is described below commit 5aaeb011f362f89caae1e1ab117c897ed2e4da21 Author: Dom G <dominic.gargu...@gmail.com> AuthorDate: Thu Apr 28 07:24:11 2022 -0400 Address TODOs found in code (#2648) Co-authored-by: Christopher Tubbs <ctubb...@apache.org> --- .../org/apache/accumulo/shell/ShellOptionsJC.java | 23 ++++++++++++++++++++-- .../apache/accumulo/test/ConditionalWriterIT.java | 9 ++++++++- .../org/apache/accumulo/test/ExistingMacIT.java | 15 +++++++------- .../accumulo/test/functional/TooManyDeletesIT.java | 2 -- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java b/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java index 09504ec2e7..aa6776074a 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java +++ b/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java @@ -32,7 +32,9 @@ import org.apache.accumulo.core.conf.ClientProperty; import org.apache.hadoop.security.UserGroupInformation; import com.beust.jcommander.DynamicParameter; +import com.beust.jcommander.IParameterValidator; import com.beust.jcommander.Parameter; +import com.beust.jcommander.ParameterException; import com.beust.jcommander.converters.FileConverter; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -110,8 +112,9 @@ public class ShellOptionsJC { private String zooKeeperHosts; @Parameter(names = "--auth-timeout", - description = "minutes the shell can be idle without re-entering a password") - private int authTimeout = 60; // TODO Add validator for positive number + description = "minutes the shell can be idle without re-entering a password", + validateWith = PositiveInteger.class) + private int authTimeout = 60; @Parameter(names = "--disable-auth-timeout", description = "disables requiring the user to re-type a password after being idle") @@ -236,4 +239,20 @@ public class ShellOptionsJC { return props; } + static class PositiveInteger implements IParameterValidator { + @Override + public void validate(String name, String value) throws ParameterException { + int n = -1; + try { + n = Integer.parseInt(value); + } catch (NumberFormatException e) { + // ignore, will be handled below + } + if (n < 0) { + throw new ParameterException( + "Parameter " + name + " should be a positive integer (was " + value + ")"); + } + } + } + } diff --git a/test/src/main/java/org/apache/accumulo/test/ConditionalWriterIT.java b/test/src/main/java/org/apache/accumulo/test/ConditionalWriterIT.java index d27f77955d..1e38f862cf 100644 --- a/test/src/main/java/org/apache/accumulo/test/ConditionalWriterIT.java +++ b/test/src/main/java/org/apache/accumulo/test/ConditionalWriterIT.java @@ -18,6 +18,7 @@ */ package org.apache.accumulo.test; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -44,6 +45,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import org.apache.accumulo.cluster.ClusterUser; import org.apache.accumulo.core.client.Accumulo; @@ -883,15 +885,20 @@ public class ConditionalWriterIT extends SharedMiniClusterBase { int count = 0; - // TODO check got each row back + Set<String> rowsReceived = new HashSet<>(); while (results.hasNext()) { Result result = results.next(); + rowsReceived.add(new String(result.getMutation().getRow(), UTF_8)); assertEquals(Status.ACCEPTED, result.getStatus()); count++; } assertEquals(num, count, "Did not receive the expected number of results"); + Set<String> rowsExpected = + rows.stream().map(row -> new String(row, UTF_8)).collect(Collectors.toSet()); + assertEquals(rowsExpected, rowsReceived, "Did not receive all expected rows"); + ArrayList<ConditionalMutation> cml2 = new ArrayList<>(num); for (int i = 0; i < num; i++) { diff --git a/test/src/main/java/org/apache/accumulo/test/ExistingMacIT.java b/test/src/main/java/org/apache/accumulo/test/ExistingMacIT.java index 6cde924490..342cb3ffb8 100644 --- a/test/src/main/java/org/apache/accumulo/test/ExistingMacIT.java +++ b/test/src/main/java/org/apache/accumulo/test/ExistingMacIT.java @@ -88,20 +88,21 @@ public class ExistingMacIT extends ConfigurableMacBase { @Test public void testExistingInstance() throws Exception { + final String rootUser = "root"; AccumuloClient client = - getCluster().createAccumuloClient("root", new PasswordToken(ROOT_PASSWORD)); + getCluster().createAccumuloClient(rootUser, new PasswordToken(ROOT_PASSWORD)); - client.tableOperations().create("table1"); + final String table = getUniqueNames(1)[0]; + client.tableOperations().create(table); - try (BatchWriter bw = client.createBatchWriter("table1")) { + try (BatchWriter bw = client.createBatchWriter(table)) { Mutation m1 = new Mutation("00081"); m1.put("math", "sqroot", "9"); m1.put("math", "sq", "6560"); bw.addMutation(m1); } - client.tableOperations().flush("table1", null, null, true); - // TODO use constants + client.tableOperations().flush(table, null, null, true); client.tableOperations().flush(MetadataTable.NAME, null, null, true); client.tableOperations().flush(RootTable.NAME, null, null, true); @@ -138,9 +139,9 @@ public class ExistingMacIT extends ConfigurableMacBase { MiniAccumuloClusterImpl accumulo2 = new MiniAccumuloClusterImpl(macConfig2); accumulo2.start(); - client = accumulo2.createAccumuloClient("root", new PasswordToken(ROOT_PASSWORD)); + client = accumulo2.createAccumuloClient(rootUser, new PasswordToken(ROOT_PASSWORD)); - try (Scanner scanner = client.createScanner("table1", Authorizations.EMPTY)) { + try (Scanner scanner = client.createScanner(table, Authorizations.EMPTY)) { int sum = 0; for (Entry<Key,Value> entry : scanner) { sum += Integer.parseInt(entry.getValue().toString()); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TooManyDeletesIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TooManyDeletesIT.java index 4fe981ee33..3af8a01b48 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/TooManyDeletesIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/TooManyDeletesIT.java @@ -47,8 +47,6 @@ public class TooManyDeletesIT extends AccumuloClusterHarness { SummarizerConfiguration sc = SummarizerConfiguration.builder(DeletesSummarizer.class).build(); - // TODO open issue about programmatic config of compaction strategies - NewTableConfiguration ntc = new NewTableConfiguration().enableSummarization(sc); HashMap<String,String> props = new HashMap<>(); props.put(Property.TABLE_COMPACTION_STRATEGY.getKey(),