> 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
> 
>

Reply via email to