Fixes #10 - Update helloworld example to new Connector builder#11
Fixes #10 - Update helloworld example to new Connector builder#11mikewalch merged 3 commits intoapache:masterfrom
Conversation
* Example also now uses conf/accumulo-client.properties * Added a log4j.properties file for examples
milleruntime
left a comment
milleruntime
left a comment
There was a problem hiding this comment.
Looks good. I think there is a lot more we could do to make examples more user friendly for newcomers but this is a good start.
|
|
||
| Connector connector = opts.getConnector(); | ||
| MultiTableBatchWriter mtbw = connector.createMultiTableBatchWriter(bwOpts.getBatchWriterConfig()); | ||
| Connector connector = Connector.builder().usingProperties("conf/accumulo-client.properties").build(); |
There was a problem hiding this comment.
+1 Not only using the new API but using a more realistic example of how to connect.
| Text colq = e.getKey().getColumnQualifier(); | ||
| log.trace("row: " + e.getKey().getRow() + ", colf: " + colf + ", colq: " + colq); | ||
| log.trace(", value: " + e.getValue().toString()); | ||
| try (Scanner scan = connector.createScanner("hellotable", Authorizations.EMPTY)) { |
There was a problem hiding this comment.
+1 Modern use of scanner.
| Text colf = new Text("colfam"); | ||
| log.trace("writing ..."); | ||
| for (int i = 0; i < 10000; i++) { | ||
| Mutation m = new Mutation(new Text(String.format("row_%d", i))); |
There was a problem hiding this comment.
Can now pass Strings to Mutation, used to could not. Changing this could be a separate PR.
| connector.tableOperations().create(opts.getTableName()); | ||
| BatchWriter bw = mtbw.getBatchWriter(opts.getTableName()); | ||
| if (!connector.tableOperations().exists("hellotable")) { | ||
| connector.tableOperations().create("hellotable"); |
There was a problem hiding this comment.
A better pattern for this is to call create and catch a table exists exception. Can do nothing when the exception is caught. Calling exists and then create is susceptible to race conditions. This could be changed in another PR.
There was a problem hiding this comment.
I think it would be good to include this race condition in the Accumulo API somehow. Perhaps we could create an overloaded method like we did with NewTableConfig. Maybe a boolean flag that won't throw an exception if the table already exists. Just a way to handle the "do nothing" exception use case.
|
|
||
| Connector connector = opts.getConnector(); | ||
|
|
||
| Scanner scan = connector.createScanner(opts.getTableName(), opts.auths); |
There was a problem hiding this comment.
Really nice to see so much code go away.
| end = new Key(new Text(opts.endKey)); | ||
| scan.setRange(new Range(start, end)); | ||
| Iterator<Entry<Key,Value>> iter = scan.iterator(); | ||
| Connector connector = Connector.builder().usingProperties("conf/accumulo-client.properties").build(); |
There was a problem hiding this comment.
When I first saw this I was thinking "maybe the props file could be args[0]". However I convinced myself that the literal string is better because its easier to understand.
There was a problem hiding this comment.
It would also make argument parsing tricky.
| } | ||
| bw.addMutation(m); | ||
| if (i % 100 == 0) | ||
| log.trace(String.valueOf(i)); |
There was a problem hiding this comment.
Why log at trace level? Why use logger instead of System.out? It seems like if we want to make the examples easy to understand then using System.out would be better than logging.
There was a problem hiding this comment.
I want users to be able to the class doing the logging. Also, logging is now automatically set up for the user so there is no risk they won't see these messages.
3 participants
I will change the README.md to tell users to configure
accumulo-client.properties. I wanted to get feedback first on these changes to helloworld before I make them to other examples and update the docs.