fluo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] ctubbsii commented on a change in pull request #960: Use NewTableConfiguration (and also trivial code cleanup)
Date Thu, 01 Jan 1970 00:00:00 GMT
ctubbsii commented on a change in pull request #960: Use NewTableConfiguration (and also trivial
code cleanup)
URL: https://github.com/apache/fluo/pull/960#discussion_r150147137
 
 

 ##########
 File path: modules/integration/src/test/java/org/apache/fluo/integration/client/FluoAdminImplIT.java
 ##########
 @@ -92,6 +102,23 @@ public void testInitializeConfig() throws Exception {
       InitializationOptions opts =
           new InitializationOptions().setClearZookeeper(true).setClearTable(true);
       admin.initialize(opts);
+
+      // verify locality groups were set on the table
+      Instance inst =
+          new ZooKeeperInstance(config.getAccumuloInstance(), config.getAccumuloZookeepers());
+      Connector conn = inst.getConnector(config.getAccumuloUser(),
+          new PasswordToken(config.getAccumuloPassword()));
+      Map<String, Set<Text>> localityGroups =
+          conn.tableOperations().getLocalityGroups(config.getAccumuloTable());
+      Assert.assertEquals("Unexpected locality group count.", 1, localityGroups.size());
+      Entry<String, Set<Text>> localityGroup = localityGroups.entrySet().iterator().next();
 
 Review comment:
   I had forgotten about `Iterables.getOnlyElement`, but had I remembered, I think I probably
would have still written it the way I did for a few reasons:
   
   Since Java 8, I've tried to make a deliberate effort to avoid Guava functional stuff in
favor of Java 8 equivalents, since all of Guava is pre-lambdas, and some of it doesn't play
nice with Java 8 functional stuff. It's also not clear to me how much Guava is going to change
to deal with Java 8 over time, so I don't know how much it makes sense to double down on Guava
usage when it isn't essential for the desired functionality. I don't want to lock in to something
which will break in future Guava versions. Iterables, notably, doesn't play nice with (isn't
easily interchangeable with) Streams, so I've intentionally avoided anything with Iterables.
So, I don't want to become too reliant on Guava in new code.
   
   I'm also hesitant to use Guava for small things when those small things on their own wouldn't
warrant the extra dependency. In this case, it's probably already on the test class path,
so that doesn't matter that much.
   
   Anyway, I don't know if any of that's a good argument against using `Iterables.getOnlyElement`
or not. It's just why I might not have used it had I remembered it existed. Maybe you can
let me know what you think. I can certainly change it.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message