> On Nov. 11, 2014, 2:05 p.m., Gwen Shapira wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java,
> >  line 46
> > <https://reviews.apache.org/r/27881/diff/1/?file=758226#file758226line46>
> >
> >     Even though I agree we should support nulls, I prefer to return empty 
> > schemas where we control the code.
> >     
> >     Do you see an issue with that?

Not against anyone returning an new Schema, but if we have more connectors 
written, it might as well be default returning null if there is no schema. I 
have made this change to have atleast the integration tests pass

Let me update the docs that this can be null, second make the base class 
methods default return null, so that it is not mandatory to override this.

I can revert the HDFS then to return this if you like it to be that way.


- Veena


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


On Nov. 11, 2014, 2:04 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27881/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2014, 2:04 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1621
>     https://issues.apache.org/jira/browse/SQOOP-1621
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see jira
> 
> The point of this ticket is to guard the connector developers.
> 
> Internally in Sqoop we can still create a empty Schema object and male sure 
> we dont have to worry about guarding everyy single place a NPE for null 
> schema can happen:)
> 
> hence the code change is in the SchemaSerialization class so that we handle 
> null schema
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> e006761 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java
>  4c6f566 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java
>  bce72b5 
>   
> execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/MRConfigurationUtils.java
>  d5f74f0 
>   
> execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestMRConfigurationUtils.java
>  fbe3e7b 
> 
> Diff: https://reviews.apache.org/r/27881/diff/
> 
> 
> Testing
> -------
> 
> yes  ( unit and integration )
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>

Reply via email to