falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Venkatesan Ramachandran" <me.venk...@gmail.com>
Subject Re: Review Request 38465: FALCON-1459 : Ability to import from database
Date Sun, 04 Oct 2015 02:59:55 GMT


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > client/src/main/resources/datasource-0.1.xsd, line 184
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083430#file1083430line184>
> >
> >     We should not have password in plain text in the xml files that can be read
via Falcon UI. This is a security risk.

There is a <passwordFile> option in the datasource that refers to a file in HDFS. This
option is exposed in order to have feature parity with Sqoop.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > client/src/main/resources/feed-0.1.xsd, line 469
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083431#file1083431line469>
> >
> >     dbname might be better than "name"

Datasource being generatic, we decided to stick with "name" to refer to the name of the datasource.
Using "dbname" makes sense only in the context of Databases, but becomes inconsistent in the
context of Datasource.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > client/src/main/resources/feed-0.1.xsd, line 517
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083431#file1083431line517>
> >
> >     If only <includes>...<includes> are specified in the feed.xml, does
import pull only the fields listed under include? Or does it pull all fields except ones listed
under <excludes>...?
> >     
> >     Please add that clarification here.

<xs:documentation>
   Specifies either an include or exclude fields list. If include field list is specified,
only
   the specified fields will be imported. If exclude field list is specified, all fields except
   the ones specified will be imported from datasource to HDFS.
</xs:documentation>


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java, line 42
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083444#file1083444line42>
> >
> >     very minor : The name HdfsClassLoader is a bit confusing. But I dont have a
better suggestion either.

Added class documentation to clarify the confusion.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java, line 731
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083448#file1083448line731>
> >
> >     The Assert in this line makes above 3 miles redundant.

It checks against NPE on those line. If line 730 fails and we want to debug, it will be easier
with this.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > common/src/test/java/org/apache/falcon/entity/parser/DatasourceEntityParserTest.java,
line 35
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083449#file1083449line35>
> >
> >     Can we add negative tests, where invalid ACL or Interfaces cause parser to throw
an exception.

These things are validated by the JAXB parser and so I think redundant.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > oozie/src/main/java/org/apache/falcon/oozie/DatabaseImportWorkflowBuilder.java,
line 41
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083458#file1083458line41>
> >
> >     This looks like SqoopImportWorkflowBuilder instead of a generic DatabaseImportWorkflowBuilder
to me. Please rename accordingly.

We are delegating the database import work to sqoop now, but it coluld change in future if
we have a differnt tool.


> On Sept. 24, 2015, 11:37 p.m., Balu Vellanki wrote:
> > common/src/test/resources/config/datasource/datasource-0.1.xml, line 46
> > <https://reviews.apache.org/r/38465/diff/3/?file=1083452#file1083452line46>
> >
> >     Please add example with properties and ACL. Please have a validity test for
these.

The properites will be used to pass in any data source specific parameters. for database import,
there is no custom properties needed and so it's empty. ACL is added to the test datasource
and the existing validation test should take care out it.


- Venkatesan


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


On Sept. 23, 2015, 11:58 p.m., Venkatesan Ramachandran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38465/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 11:58 p.m.)
> 
> 
> Review request for Falcon, Ajay Yadava, Balu Vellanki, Peeyush Bishnoi, Sowmya Ramesh,
and Venkat Ranganathan.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-1459 : Ability to import from database
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/LifeCycle.java 58a2a6c 
>   client/src/main/java/org/apache/falcon/Tag.java beeb812 
>   client/src/main/java/org/apache/falcon/entity/v0/EntityType.java 0657124 
>   client/src/main/java/org/apache/falcon/metadata/RelationshipType.java f034772 
>   client/src/main/resources/datasource-0.1.xsd PRE-CREATION 
>   client/src/main/resources/feed-0.1.xsd 2af28d2 
>   client/src/main/resources/jaxb-binding.xjb 6f1d6c7 
>   client/src/main/resources/mysql_database.xml PRE-CREATION 
>   common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java ad41674 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
>   common/src/main/java/org/apache/falcon/entity/parser/DatasourceEntityParser.java PRE-CREATION

>   common/src/main/java/org/apache/falcon/entity/parser/EntityParserFactory.java 5a33201

>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 4f5599e

>   common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java e27187b

>   common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java bd4c6cf 
>   common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java bd32852

>   common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java
8c3876c 
>   common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java 4454239

>   common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java e9946c4 
>   common/src/test/java/org/apache/falcon/entity/EntityTypeTest.java 640e87d 
>   common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java c70cfcc 
>   common/src/test/java/org/apache/falcon/entity/parser/DatasourceEntityParserTest.java
PRE-CREATION 
>   common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java b6fdb13

>   common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java 3863b11 
>   common/src/test/resources/config/datasource/datasource-0.1.xml PRE-CREATION 
>   common/src/test/resources/config/datasource/datasource-invalid-0.1.xml PRE-CREATION

>   common/src/test/resources/config/feed/feed-import-0.1.xml PRE-CREATION 
>   common/src/test/resources/config/feed/feed-import-invalid-0.1.xml PRE-CREATION 
>   docs/src/site/twiki/EntitySpecification.twiki d4f4140 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HiveAssert.java
2a934b5 
>   oozie/src/main/java/org/apache/falcon/oozie/DatabaseImportWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/FeedImportCoordinatorBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/ImportWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/OozieCoordinatorBuilder.java a04ae95 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java
3213a70 
>   oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java b819dee 
>   oozie/src/main/resources/action/feed/import-sqoop-database-action.xml PRE-CREATION

>   pom.xml 646de69 
>   webapp/pom.xml 828f7f5 
>   webapp/src/test/java/org/apache/falcon/lifecycle/FeedImportIT.java PRE-CREATION 
>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java d067dee 
>   webapp/src/test/java/org/apache/falcon/util/HsqldbTestUtils.java PRE-CREATION 
>   webapp/src/test/resources/datasource-template.xml PRE-CREATION 
>   webapp/src/test/resources/feed-template3.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38465/diff/
> 
> 
> Testing
> -------
> 
> * Unit tests
> * Integration tests
> * Manual tests
>   * Setup MySQL, create table and populate
>   * Create datasource and feed entity with import policy in Falcon  
>   * Made sure the data lands up in the HDFS.
> 
> 
> Thanks,
> 
> Venkatesan Ramachandran
> 
>


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