sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Szabolcs Vasas <vasas.szabo...@gmail.com>
Subject Re: Review Request 53423: SQOOP-2349 Transaction isolation level for metadata queries should be mutable
Date Fri, 04 Nov 2016 14:17:41 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53423/#review154901
-----------------------------------------------------------


Ship it!




Hi Attila,

The fix looks good to me however I have an idea which may be an overkill to add now but I
think it could make the code a bit simpler.
We could introduce an enum for the isolation levels, which would encapsulate the name and
the int code of the isolation. Using this enum we could eliminate the switch statement from
the BaseSqoopTool and the unit test could be also simpler. What do you think?

- Szabolcs Vasas


On Nov. 4, 2016, 3:52 a.m., Attila Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53423/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2016, 3:52 a.m.)
> 
> 
> Review request for Sqoop, Abraham Elmahrek, Abraham Fine, Boglarka Egyed, Anna Szonyi,
Szabolcs Vasas, and Erzsebet Szilagyi.
> 
> 
> Bugs: SQOOP-2349
>     https://issues.apache.org/jira/browse/SQOOP-2349
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> I've introduced a cmd line parameter for being able to set the metadata transaction levels
(defined on java.sql.Connection) manually if necessary. The change is backward compatible,
so by default SQOOP gonna still use READ_COMMITTED.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/SqoopOptions.java e14a0b7 
>   src/java/org/apache/sqoop/manager/SqlManager.java 768507b 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 13a9697 
>   src/test/org/apache/sqoop/tool/TestImportTool.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53423/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Attila Szabo
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message