Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Re: Review Request 8559: Add Accumulo support to Sqoop

Copy link to this message
Re: Review Request 8559: Add Accumulo support to Sqoop

This is an automatically generated e-mail. To reply, visit:


    Could these go in a central place? Either in a accumulo.Constants calss or AccumuloUtil?


    Just curious, is there a reason you went for throwing an error rather than using the equivalent of having EMPTY_BYTES for these, the way the convenience methods in Mutation (http://bit.ly/16VR93I) handle a missing visibility?


    When ACCUMULO_TEST_MODE is set, won't all of this error out?
    In the event that the job set up uses the test mode to create a MAC (and sets the ZK and instance names correctly), then the line getting the test mode option isn't needed.


    Recommend Iterable<Mutation> instead of List, so that other implementations have an easier time not materializing the whole list in memory.


    Doesn't this fail in ACCUMULO_TEST_MODE?
    This looks like the place where we should set up the MAC, presuming we can overwrite the Accumulo connection information.


    If I specified ACCUMULO_TEST_MODE, I don't need to provide these options, yes?


    Could you add a test class that ensures that when we use ACCUMULO_TEST_MODE, nothing is written to the accumulo instance specified in the connection arguments?


    It looks like Sqoop uses JUnit 4.11 to run tests, which means they could be run in parallel (though I don't know if Sqoop is usually built this way).
    At a minimum, this needs a strong warning about not being thread safe.
    it should probably be rewritten with a lock. and maybe an AtomicInteger, which you could use to do reference counting so that stopping can happen in the tearDown method.
- Sean Busbey
On Oct. 29, 2013, 9:22 p.m., Philip Grim wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8559/
> -----------------------------------------------------------
> (Updated Oct. 29, 2013, 9:22 p.m.)
> Review request for accumulo, Sqoop and Jarek Cecho.
> Repository: sqoop-trunk
> Description
> -------
> Adds the ability to import to an Accumulo table in much the same manner as the current HBase import capability.  Reported in JIRA as ACCUMULO-141 and SQOOP-767.
> Diffs
> -----
>   ivy.xml c5130ae
>   src/docs/user/accumulo-args.txt PRE-CREATION
>   src/docs/user/accumulo.txt PRE-CREATION
>   src/java/org/apache/sqoop/SqoopOptions.java 13637b5
>   src/java/org/apache/sqoop/accumulo/AccumuloMutationProcessor.java PRE-CREATION
>   src/java/org/apache/sqoop/accumulo/AccumuloUtil.java PRE-CREATION
>   src/java/org/apache/sqoop/accumulo/MutationTransformer.java PRE-CREATION
>   src/java/org/apache/sqoop/accumulo/ToStringMutationTransformer.java PRE-CREATION
>   src/java/org/apache/sqoop/manager/SqlManager.java 1ffa40f
>   src/java/org/apache/sqoop/mapreduce/AccumuloImportJob.java PRE-CREATION