You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Veena Basavaraj <vb...@cloudera.com> on 2014/12/03 22:23:05 UTC
Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-1839
https://issues.apache.org/jira/browse/SQOOP-1839
Repository: sqoop-sqoop2
Description
-------
see jira
Diffs
-----
common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
Diff: https://reviews.apache.org/r/28676/diff/
Testing
-------
Thanks,
Veena Basavaraj
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 4, 2014, 1:02 a.m., Abraham Elmahrek wrote:
> > core/src/main/java/org/apache/sqoop/driver/JobManager.java, lines 579-582
> > <https://reviews.apache.org/r/28676/diff/2/?file=782081#file782081line579>
> >
> > The variable naming here is a bit confusing to me. submissionId and externalId seem different. 'submissionId' seems like it would be used in Sqoop and 'externalId' seems like it would be used outside of sqoop.
changed it back. it is infact a job Id, but external. persistenceId is the submisisonid
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63825
-----------------------------------------------------------
On Dec. 3, 2014, 1:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 1:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 4, 2014, 1:02 a.m., Abraham Elmahrek wrote:
> > core/src/main/java/org/apache/sqoop/driver/JobManager.java, lines 579-582
> > <https://reviews.apache.org/r/28676/diff/2/?file=782081#file782081line579>
> >
> > The variable naming here is a bit confusing to me. submissionId and externalId seem different. 'submissionId' seems like it would be used in Sqoop and 'externalId' seems like it would be used outside of sqoop.
>
> Veena Basavaraj wrote:
> changed it back. it is infact a job Id, but external. persistenceId is the submisisonid
Thinking a bit more, the reason I changed this in the SubmissionEngine API as well, since the api expects a submission Id.
So lets be consistent and call it the jobId, since externalId is a jobId
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63825
-----------------------------------------------------------
On Dec. 3, 2014, 1:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 1:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63825
-----------------------------------------------------------
common/src/main/java/org/apache/sqoop/json/SubmissionBean.java
<https://reviews.apache.org/r/28676/#comment106137>
failure-info?
core/src/main/java/org/apache/sqoop/driver/JobManager.java
<https://reviews.apache.org/r/28676/#comment106136>
The variable naming here is a bit confusing to me. submissionId and externalId seem different. 'submissionId' seems like it would be used in Sqoop and 'externalId' seems like it would be used outside of sqoop.
- Abraham Elmahrek
On Dec. 3, 2014, 9:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 9:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Jarek Cecho <ja...@apache.org>.
> On Dec. 4, 2014, 3:24 p.m., Jarek Cecho wrote:
> > I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
> >
> > We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
> >
> > 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
> > 2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
>
> Veena Basavaraj wrote:
> it is not an exception. Failure and Exceptions should be treated diffeently.
>
> Jarek Cecho wrote:
> Hadoop is actually usually returning a StackTrace as part of the "failure" call, so they are pretty much equivalent as far as I know.
>
> I don't think that it make sense to have two entities for different error states for external system when only one of them can be possibly filled at the same time. If we don't like the name, we could rename it to something more general, but having two entries doesn't seem needed. It's an external system after all, so putting any semantics to that error might break on next Hadoop (or Spark) version.
>
> Veena Basavaraj wrote:
> Does failure Info have the stack trace? If yes, if it is all in one field then great.
>
> I am fine with one field, as long as it is geeneric name and we can have one field. What is info and stack trace separately buying us?
Talking from my experience, then yes the failure info usually contains stack trace (that is quite frequently truncated though as they have very small fixed size buffer for the exception).
The idea for having separate exceptionInfo and exceptionStack trace was primary in presentation. The info should be brief (one line), whereas the exceptionStackTrace is entire multi-line exception trace. For example we're putting the exception message into "exceptionInfo" and stacktrace into stacktrace.
As the getFailureInfo is giving us only one thing, we might either put some generic text into the exceptionInfo or copy there the first line from getFailureInfo call. I'm sure that there are other ideas as well :)
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
On Dec. 3, 2014, 9:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 9:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Jarek Cecho <ja...@apache.org>.
> On Dec. 4, 2014, 3:24 p.m., Jarek Cecho wrote:
> > I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
> >
> > We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
> >
> > 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
> > 2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
>
> Veena Basavaraj wrote:
> it is not an exception. Failure and Exceptions should be treated diffeently.
Hadoop is actually usually returning a StackTrace as part of the "failure" call, so they are pretty much equivalent as far as I know.
I don't think that it make sense to have two entities for different error states for external system when only one of them can be possibly filled at the same time. If we don't like the name, we could rename it to something more general, but having two entries doesn't seem needed. It's an external system after all, so putting any semantics to that error might break on next Hadoop (or Spark) version.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
On Dec. 3, 2014, 9:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 9:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 4, 2014, 7:24 a.m., Jarek Cecho wrote:
> > I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
> >
> > We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
> >
> > 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
> > 2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
>
> Veena Basavaraj wrote:
> it is not an exception. Failure and Exceptions should be treated diffeently.
>
> Jarek Cecho wrote:
> Hadoop is actually usually returning a StackTrace as part of the "failure" call, so they are pretty much equivalent as far as I know.
>
> I don't think that it make sense to have two entities for different error states for external system when only one of them can be possibly filled at the same time. If we don't like the name, we could rename it to something more general, but having two entries doesn't seem needed. It's an external system after all, so putting any semantics to that error might break on next Hadoop (or Spark) version.
Does failure Info have the stack trace? If yes, if it is all in one field then great.
I am fine with one field, as long as it is geeneric name and we can have one field. What is info and stack trace separately buying us?
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
On Dec. 3, 2014, 1:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 1:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 4, 2014, 7:24 a.m., Jarek Cecho wrote:
> > I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
> >
> > We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
> >
> > 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
> > 2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
>
> Veena Basavaraj wrote:
> it is not an exception. Failure and Exceptions should be treated diffeently.
>
> Jarek Cecho wrote:
> Hadoop is actually usually returning a StackTrace as part of the "failure" call, so they are pretty much equivalent as far as I know.
>
> I don't think that it make sense to have two entities for different error states for external system when only one of them can be possibly filled at the same time. If we don't like the name, we could rename it to something more general, but having two entries doesn't seem needed. It's an external system after all, so putting any semantics to that error might break on next Hadoop (or Spark) version.
>
> Veena Basavaraj wrote:
> Does failure Info have the stack trace? If yes, if it is all in one field then great.
>
> I am fine with one field, as long as it is geeneric name and we can have one field. What is info and stack trace separately buying us?
>
> Jarek Cecho wrote:
> Talking from my experience, then yes the failure info usually contains stack trace (that is quite frequently truncated though as they have very small fixed size buffer for the exception).
>
> The idea for having separate exceptionInfo and exceptionStack trace was primary in presentation. The info should be brief (one line), whereas the exceptionStackTrace is entire multi-line exception trace. For example we're putting the exception message into "exceptionInfo" and stacktrace into stacktrace.
>
> As the getFailureInfo is giving us only one thing, we might either put some generic text into the exceptionInfo or copy there the first line from getFailureInfo call. I'm sure that there are other ideas as well :)
ok. works.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
On Dec. 3, 2014, 1:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 1:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 4, 2014, 7:24 a.m., Jarek Cecho wrote:
> > I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
> >
> > We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
> >
> > 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
> > 2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
it is not an exception. Failure and Exceptions should be treated diffeently.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
On Dec. 3, 2014, 1:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 1:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review63851
-----------------------------------------------------------
I'm wondering what are your thoughts for adding a new entry into the MSubmission class Veena?
We already do have two entries associated with exceptions from the Job execution - exceptionInfo and exceptionStackTrace [1]. As far as I can tell we're already using them to propage exceptions from the execution engine on job submission [2], so I'm wondering why not use the same entries for exceptions that occurs post submission?
1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MSubmission.java#L90
2: https://github.com/apache/sqoop/blob/sqoop2/submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java#L255
- Jarek Cecho
On Dec. 3, 2014, 9:34 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2014, 9:34 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 10, 2014, 8:44 a.m., Gwen Shapira wrote:
> > Thank you so much for fixing it.
> >
> > Few comments:
> > * The change modifies the JSON returned when getting submission from REST API. I'd expect a documentation change too.
> > * SubmissionBean changes, so the client API changed too - again, we need to document the new behavior
> > * I'm concerned about the repository changes - what will happen with submissions already stored in the DB? Is the API change backwards compatible? Will be be able to read old submissions? If it is, perhaps add a unit test to show compatibility?
> > * The tests are only for exceptions and traces, but I thought part of the change was to support errors without exceptions? Don't we need to test those?
>
> Veena Basavaraj wrote:
> - back compat with JSON is not been maintained since many releases and I dont know if this matters much since the only client is HUE and it is not even ready
>
> - happy to add a doc ticket for the naming change once the code is in I can fix the doc
>
> - the schema will still store the same 2 fields in the existing table, might be a good idea to rename the fields in
> - | SQS_EXCEPTION: VARCHAR(150) |
> * | SQS_EXCEPTION_TRACE: VARCHAR(750) |
> * +----------------------------------
>
>
> The change unifies the failure and exception into one thing called error. So not sure what testing is required there
>
>
> The integration tests pass so this means the fields are stored and retrieved in the submission, not sure unti tests for compatibility check makes sense, have not done that, would you mind iterating your thoguhts on that?
>
> Gwen Shapira wrote:
> * I'm ok with not maintaining back compat for JSON for now.
>
> * The backward compat I'm concerned about is: What will happen if we have existing submissions in the DB and use the new API to request them? will we be able to read them? I'd like to see a test around that.
>
> * Any reason not to add doc changes in this patch? It gives us "atomicity" for the change. (this is not specific for this patch, I insist on docs inclusion in all patches I review. I think its the right practice)
>
> * It will be clearer to rename, but then we'll have to worry about upgrades. I'll let you make the call here.
>
> * failure and exception are integrated into one error. I'd like to see tests (if possible) showing that we successfully grab non-exception failures into the field. I'm not sure we have those right now.
>
> Hope this is clearer?
>
> Veena Basavaraj wrote:
> I 'd rather have separate tickets and patches for doc and schema renames
>
> As I said before integration tests passes, so this means it can read and write a submission. Let me know if you need more details on how the integration is testing the submission code path.
>
> rename means upgrade as well, its implicit since I have tons of this in the past.:)
>
> can you elaborate which part ? is that a unit or integration test, since the failure is a MR job related data. I must be missing soemthing there?
changed my mind:) fixed the .rst as well
added ticket for schema rename and upgrade
ok followed up on testing filed 2 more tickets to follow up, they will have tests, since UNIT tests are lacking in general for any functionality in MR submission engine
https://issues.apache.org/jira/browse/SQOOP-1877
https://issues.apache.org/jira/browse/SQOOP-1878
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64563
-----------------------------------------------------------
On Dec. 10, 2014, 1:13 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2014, 1:13 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> docs/src/site/sphinx/RESTAPI.rst c0c236e
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 10, 2014, 8:44 a.m., Gwen Shapira wrote:
> > Thank you so much for fixing it.
> >
> > Few comments:
> > * The change modifies the JSON returned when getting submission from REST API. I'd expect a documentation change too.
> > * SubmissionBean changes, so the client API changed too - again, we need to document the new behavior
> > * I'm concerned about the repository changes - what will happen with submissions already stored in the DB? Is the API change backwards compatible? Will be be able to read old submissions? If it is, perhaps add a unit test to show compatibility?
> > * The tests are only for exceptions and traces, but I thought part of the change was to support errors without exceptions? Don't we need to test those?
>
> Veena Basavaraj wrote:
> - back compat with JSON is not been maintained since many releases and I dont know if this matters much since the only client is HUE and it is not even ready
>
> - happy to add a doc ticket for the naming change once the code is in I can fix the doc
>
> - the schema will still store the same 2 fields in the existing table, might be a good idea to rename the fields in
> - | SQS_EXCEPTION: VARCHAR(150) |
> * | SQS_EXCEPTION_TRACE: VARCHAR(750) |
> * +----------------------------------
>
>
> The change unifies the failure and exception into one thing called error. So not sure what testing is required there
>
>
> The integration tests pass so this means the fields are stored and retrieved in the submission, not sure unti tests for compatibility check makes sense, have not done that, would you mind iterating your thoguhts on that?
>
> Gwen Shapira wrote:
> * I'm ok with not maintaining back compat for JSON for now.
>
> * The backward compat I'm concerned about is: What will happen if we have existing submissions in the DB and use the new API to request them? will we be able to read them? I'd like to see a test around that.
>
> * Any reason not to add doc changes in this patch? It gives us "atomicity" for the change. (this is not specific for this patch, I insist on docs inclusion in all patches I review. I think its the right practice)
>
> * It will be clearer to rename, but then we'll have to worry about upgrades. I'll let you make the call here.
>
> * failure and exception are integrated into one error. I'd like to see tests (if possible) showing that we successfully grab non-exception failures into the field. I'm not sure we have those right now.
>
> Hope this is clearer?
I 'd rather have separate tickets and patches for doc and schema renames
As I said before integration tests passes, so this means it can read and write a submission. Let me know if you need more details on how the integration is testing the submission code path.
rename means upgrade as well, its implicit since I have tons of this in the past.:)
can you elaborate which part ? is that a unit or integration test, since the failure is a MR job related data. I must be missing soemthing there?
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64563
-----------------------------------------------------------
On Dec. 8, 2014, 2:40 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 2:40 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Dec. 10, 2014, 4:44 p.m., Gwen Shapira wrote:
> > Thank you so much for fixing it.
> >
> > Few comments:
> > * The change modifies the JSON returned when getting submission from REST API. I'd expect a documentation change too.
> > * SubmissionBean changes, so the client API changed too - again, we need to document the new behavior
> > * I'm concerned about the repository changes - what will happen with submissions already stored in the DB? Is the API change backwards compatible? Will be be able to read old submissions? If it is, perhaps add a unit test to show compatibility?
> > * The tests are only for exceptions and traces, but I thought part of the change was to support errors without exceptions? Don't we need to test those?
>
> Veena Basavaraj wrote:
> - back compat with JSON is not been maintained since many releases and I dont know if this matters much since the only client is HUE and it is not even ready
>
> - happy to add a doc ticket for the naming change once the code is in I can fix the doc
>
> - the schema will still store the same 2 fields in the existing table, might be a good idea to rename the fields in
> - | SQS_EXCEPTION: VARCHAR(150) |
> * | SQS_EXCEPTION_TRACE: VARCHAR(750) |
> * +----------------------------------
>
>
> The change unifies the failure and exception into one thing called error. So not sure what testing is required there
>
>
> The integration tests pass so this means the fields are stored and retrieved in the submission, not sure unti tests for compatibility check makes sense, have not done that, would you mind iterating your thoguhts on that?
* I'm ok with not maintaining back compat for JSON for now.
* The backward compat I'm concerned about is: What will happen if we have existing submissions in the DB and use the new API to request them? will we be able to read them? I'd like to see a test around that.
* Any reason not to add doc changes in this patch? It gives us "atomicity" for the change. (this is not specific for this patch, I insist on docs inclusion in all patches I review. I think its the right practice)
* It will be clearer to rename, but then we'll have to worry about upgrades. I'll let you make the call here.
* failure and exception are integrated into one error. I'd like to see tests (if possible) showing that we successfully grab non-exception failures into the field. I'm not sure we have those right now.
Hope this is clearer?
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64563
-----------------------------------------------------------
On Dec. 8, 2014, 10:40 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 10:40 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Dec. 10, 2014, 8:44 a.m., Gwen Shapira wrote:
> > Thank you so much for fixing it.
> >
> > Few comments:
> > * The change modifies the JSON returned when getting submission from REST API. I'd expect a documentation change too.
> > * SubmissionBean changes, so the client API changed too - again, we need to document the new behavior
> > * I'm concerned about the repository changes - what will happen with submissions already stored in the DB? Is the API change backwards compatible? Will be be able to read old submissions? If it is, perhaps add a unit test to show compatibility?
> > * The tests are only for exceptions and traces, but I thought part of the change was to support errors without exceptions? Don't we need to test those?
- back compat with JSON is not been maintained since many releases and I dont know if this matters much since the only client is HUE and it is not even ready
- happy to add a doc ticket for the naming change once the code is in I can fix the doc
- the schema will still store the same 2 fields in the existing table, might be a good idea to rename the fields in
- | SQS_EXCEPTION: VARCHAR(150) |
* | SQS_EXCEPTION_TRACE: VARCHAR(750) |
* +----------------------------------
The change unifies the failure and exception into one thing called error. So not sure what testing is required there
The integration tests pass so this means the fields are stored and retrieved in the submission, not sure unti tests for compatibility check makes sense, have not done that, would you mind iterating your thoguhts on that?
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64563
-----------------------------------------------------------
On Dec. 8, 2014, 2:40 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 2:40 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64563
-----------------------------------------------------------
Thank you so much for fixing it.
Few comments:
* The change modifies the JSON returned when getting submission from REST API. I'd expect a documentation change too.
* SubmissionBean changes, so the client API changed too - again, we need to document the new behavior
* I'm concerned about the repository changes - what will happen with submissions already stored in the DB? Is the API change backwards compatible? Will be be able to read old submissions? If it is, perhaps add a unit test to show compatibility?
* The tests are only for exceptions and traces, but I thought part of the change was to support errors without exceptions? Don't we need to test those?
- Gwen Shapira
On Dec. 8, 2014, 10:40 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 10:40 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64704
-----------------------------------------------------------
Ship it!
Ship It!
- Abraham Elmahrek
On Dec. 10, 2014, 9:13 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2014, 9:13 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> docs/src/site/sphinx/RESTAPI.rst c0c236e
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/#review64658
-----------------------------------------------------------
Ship it!
LGTM.
Non-binding +1.
- Gwen Shapira
On Dec. 10, 2014, 9:13 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28676/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2014, 9:13 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1839
> https://issues.apache.org/jira/browse/SQOOP-1839
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
> common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
> common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
> common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
> core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
> core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
> core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
> docs/src/site/sphinx/RESTAPI.rst c0c236e
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
> shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
> shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
> test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
>
> Diff: https://reviews.apache.org/r/28676/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/
-----------------------------------------------------------
(Updated Dec. 10, 2014, 1:13 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1839
https://issues.apache.org/jira/browse/SQOOP-1839
Repository: sqoop-sqoop2
Description
-------
see jira
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
docs/src/site/sphinx/RESTAPI.rst c0c236e
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
Diff: https://reviews.apache.org/r/28676/diff/
Testing
-------
Thanks,
Veena Basavaraj
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/
-----------------------------------------------------------
(Updated Dec. 8, 2014, 2:40 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1839
https://issues.apache.org/jira/browse/SQOOP-1839
Repository: sqoop-sqoop2
Description
-------
see jira
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
common/src/main/java/org/apache/sqoop/model/SubmissionError.java PRE-CREATION
common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 4f5f52a
Diff: https://reviews.apache.org/r/28676/diff/
Testing
-------
Thanks,
Veena Basavaraj
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/
-----------------------------------------------------------
(Updated Dec. 8, 2014, 1:20 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1839
https://issues.apache.org/jira/browse/SQOOP-1839
Repository: sqoop-sqoop2
Description
-------
see jira
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
common/src/main/java/org/apache/sqoop/model/JobSubmissionError.java PRE-CREATION
common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d07eda9
core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561
core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java e2e8073
shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 4a0f476
shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java 0e2a38d
submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
Diff: https://reviews.apache.org/r/28676/diff/
Testing
-------
Thanks,
Veena Basavaraj
Re: Review Request 28676: Sqoop2: Mapreduce submission record should
propogate failure info as well
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28676/
-----------------------------------------------------------
(Updated Dec. 3, 2014, 1:34 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1839
https://issues.apache.org/jira/browse/SQOOP-1839
Repository: sqoop-sqoop2
Description
-------
see jira
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/json/SubmissionBean.java e926f02
common/src/main/java/org/apache/sqoop/model/MSubmission.java 2648712
core/src/main/java/org/apache/sqoop/driver/JobManager.java d6efa6d
core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5
core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f
submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java 631ceca
Diff: https://reviews.apache.org/r/28676/diff/
Testing
-------
Thanks,
Veena Basavaraj