ctubbsii commented on a change in pull request #337: ACCUMULO-4732 No APIs to configure iterators or locality groups for new tables URL: https://github.com/apache/accumulo/pull/337#discussion_r158156098
########## File path: core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java ########## @@ -146,4 +163,91 @@ public NewTableConfiguration enableSummarization(SummarizerConfiguration... conf summarizerProps = tmp; return this; } + + /** + * Configures a table's locality groups prior to initial table creation. + * + * Allows locality groups to be set prior to table creation. Additional calls to this method prior to table creation will overwrite previous locality group + * mappings. + * + * @param groups + * mapping of locality group names to column families in the locality group + * + * @since 2.0.0 + * + * @see TableOperations#setLocalityGroups + */ + public NewTableConfiguration setLocalityGroups(Map<String,Set<Text>> groups) { + // ensure locality groups do not overlap + LocalityGroupUtil.ensureNonOverlappingGroups(groups); + Map<String,String> tmp = new HashMap<>(); + for (Entry<String,Set<Text>> entry : groups.entrySet()) { + Set<Text> colFams = entry.getValue(); + String value = LocalityGroupUtil.encodeColumnFamilies(colFams); + tmp.put(Property.TABLE_LOCALITY_GROUP_PREFIX + entry.getKey(), value); + } + tmp.put(Property.TABLE_LOCALITY_GROUPS.getKey(), groups.keySet().stream().collect(Collectors.joining(","))); + checkDisjoint(properties, tmp, "locality groups"); + localityProps = tmp; + return this; + } + + /** + * Configure iterator settings for a table prior to its creation. + * + * Additional calls to this method before table creation will overwrite previous iterator settings. + * + * @param setting + * object specifying the properties of the iterator + * @throws AccumuloException + * if a general error occurs + * + * @since 2.0.0 + * + * @see TableOperations#attachIterator(String, IteratorSetting) + */ + public NewTableConfiguration attachIterator(IteratorSetting setting) throws AccumuloException { Review comment: I don't think that these methods need to throw a checked `AccumuloException`. It might be preferable for them to just throw an `IllegalArgumentException`, since the problem is with the bad argument to the method, and not a problem with Accumulo itself. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services