You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2013/03/07 17:28:30 UTC
Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
I've used following command to change all error exception logging to call method that properly resolves "getNextException":
grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
This addresses bug SQOOP-933.
https://issues.apache.org/jira/browse/SQOOP-933
Diffs
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java d270bc87dd793a0c7f30888571e8048a98a7f707
src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
Diff: https://reviews.apache.org/r/9803/diff/
Testing
-------
Unit tests are passing.
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/#review17646
-----------------------------------------------------------
Ship it!
Thanks for taking care of the additional file Jarcec
- Venkat Ranganathan
On March 9, 2013, 5:48 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9803/
> -----------------------------------------------------------
>
> (Updated March 9, 2013, 5:48 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've used following command to change all error exception logging to call method that properly resolves "getNextException":
>
> grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
>
>
> This addresses bug SQOOP-933.
> https://issues.apache.org/jira/browse/SQOOP-933
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
> src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
> src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
> src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
> src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
> src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
> src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
> src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
> src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
>
> Diff: https://reviews.apache.org/r/9803/diff/
>
>
> Testing
> -------
>
> Unit tests are passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Abhijeet Gaikwad <ab...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/#review17676
-----------------------------------------------------------
Ship it!
Unit tests pass.
- Abhijeet Gaikwad
On March 9, 2013, 5:48 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9803/
> -----------------------------------------------------------
>
> (Updated March 9, 2013, 5:48 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've used following command to change all error exception logging to call method that properly resolves "getNextException":
>
> grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
>
>
> This addresses bug SQOOP-933.
> https://issues.apache.org/jira/browse/SQOOP-933
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
> src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
> src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
> src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
> src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
> src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
> src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
> src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
> src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
>
> Diff: https://reviews.apache.org/r/9803/diff/
>
>
> Testing
> -------
>
> Unit tests are passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/
-----------------------------------------------------------
(Updated March 9, 2013, 5:48 p.m.)
Review request for Sqoop.
Changes
-------
Removed extra file that was left there from different patch.
Description
-------
I've used following command to change all error exception logging to call method that properly resolves "getNextException":
grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
This addresses bug SQOOP-933.
https://issues.apache.org/jira/browse/SQOOP-933
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
Diff: https://reviews.apache.org/r/9803/diff/
Testing
-------
Unit tests are passing.
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/#review17549
-----------------------------------------------------------
Ship it!
LGTM.
There is one additional change on password key management into Credentials. Do you plan to add another JIRA for this and mark as resolved by this patch?
Thanks
Venkat
- Venkat Ranganathan
On March 7, 2013, 4:28 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9803/
> -----------------------------------------------------------
>
> (Updated March 7, 2013, 4:28 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've used following command to change all error exception logging to call method that properly resolves "getNextException":
>
> grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
>
>
> This addresses bug SQOOP-933.
> https://issues.apache.org/jira/browse/SQOOP-933
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
> src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
> src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
> src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
> src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
> src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java d270bc87dd793a0c7f30888571e8048a98a7f707
> src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
> src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
> src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
> src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
>
> Diff: https://reviews.apache.org/r/9803/diff/
>
>
> Testing
> -------
>
> Unit tests are passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Jarek Cecho <ja...@apache.org>.
> On March 8, 2013, 2:33 a.m., Abhijeet Gaikwad wrote:
> > Few clarifications:
> > 1. Few logging levels changed from warning to error. If those are really some warnings we shouldn't log them as error.
> > 2. As Venkat pointed out - are we merging two different patches?
Thank you both Abhijeet and Venkat for the reviews. The extra file is indeed my overlook and definitely should not be there. I'll upload fixed patch shortly, thank you very much for catching this up!
I've intentionally changed few warnings into errors. My justification is that exactly the same errors (exceptions from closing ResultSet, ...) are somewhere logged as error and somewhere else as warning, so I wanted to unify the behavior across the code base. I believe that even though they most likely will not affect functionality they are real errors and should be logged accordingly.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/#review17591
-----------------------------------------------------------
On March 7, 2013, 4:28 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9803/
> -----------------------------------------------------------
>
> (Updated March 7, 2013, 4:28 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've used following command to change all error exception logging to call method that properly resolves "getNextException":
>
> grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
>
>
> This addresses bug SQOOP-933.
> https://issues.apache.org/jira/browse/SQOOP-933
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
> src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
> src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
> src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
> src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
> src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java d270bc87dd793a0c7f30888571e8048a98a7f707
> src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
> src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
> src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
> src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
>
> Diff: https://reviews.apache.org/r/9803/diff/
>
>
> Testing
> -------
>
> Unit tests are passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-933 Properly log SQLExceptions using
getNextException() method for chaining exceptions
Posted by Abhijeet Gaikwad <ab...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9803/#review17591
-----------------------------------------------------------
Few clarifications:
1. Few logging levels changed from warning to error. If those are really some warnings we shouldn't log them as error.
2. As Venkat pointed out - are we merging two different patches?
- Abhijeet Gaikwad
On March 7, 2013, 4:28 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9803/
> -----------------------------------------------------------
>
> (Updated March 7, 2013, 4:28 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've used following command to change all error exception logging to call method that properly resolves "getNextException":
>
> grep "(SQLException" src/java -R -A 2 -n | grep "LOG.error"
>
>
> This addresses bug SQOOP-933.
> https://issues.apache.org/jira/browse/SQOOP-933
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java fa7661e38e5cf0fd9ad202d3953e7b64f8a4bf8c
> src/java/org/apache/sqoop/manager/Db2Manager.java a84b4dcd2b7101f0eb7bb4ea30e104c7aad6d677
> src/java/org/apache/sqoop/manager/MySQLManager.java b79c92664462e80bb6e1028cf14d569e805f0bbf
> src/java/org/apache/sqoop/manager/OracleManager.java 56244c741d9221b01cab27454bb18be4937ff639
> src/java/org/apache/sqoop/manager/SqlManager.java f0a920df4d423ec0caa4efb34cd7678a8b8f54b9
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 5261671bdc3461f57fa3171d12447aeba2079444
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java 6c8d8068e73bc844b309f62726ec4fb9543abc67
> src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java 63c52c7bedfdea6fb3d85642dc8f9cb78b1a1e07
> src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java d270bc87dd793a0c7f30888571e8048a98a7f707
> src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java cd37a25e1a06dc34c8066370c53df0161796a093
> src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java ab8bf1894149e50e4233113b112832ef87c0fe06
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 81677d7133f91acec5961ca4d91dd8f1575b6395
> src/java/org/apache/sqoop/util/LoggingUtils.java cb74401be65cf181fc0000e7cef45982b99eeae2
> src/java/org/apache/sqoop/util/ResultSetPrinter.java c6ec09df70c8643e1b8b5d26e4248cefcacded59
>
> Diff: https://reviews.apache.org/r/9803/diff/
>
>
> Testing
> -------
>
> Unit tests are passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>