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