----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38465/#review100491 -----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java (line 55) <https://reviews.apache.org/r/38465/#comment157696> feedCluster.getImport() can return null as import minoccurs is 0? common/src/main/java/org/apache/falcon/entity/parser/DatasourceEntityParser.java (line 66) <https://reviews.apache.org/r/38465/#comment157699> minot nit: Can these validate methods be static if possible? common/src/main/java/org/apache/falcon/entity/parser/DatasourceEntityParser.java (line 71) <https://reviews.apache.org/r/38465/#comment157700> minot nit: use StringUtils.isNotBlank()? common/src/main/java/org/apache/falcon/entity/parser/DatasourceEntityParser.java (line 72) <https://reviews.apache.org/r/38465/#comment157703> For logging instead of using string.format we use use {}. Please fix all the logging. e.g.: LOG.info("The new entry is {}.", entry); common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java (line 68) <https://reviews.apache.org/r/38465/#comment157739> I don't see the code changes for adding metadata for instances. Please add it. common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java (line 49) <https://reviews.apache.org/r/38465/#comment157726> Please use {} instead of + for logging. e.g.: Instead of logger.debug("The new entry is "+entry+"."); use logger.debug("The new entry is {}.", entry); second form will outperform the first form by a factor of at least 30, in case of a disabled logging statement. http://www.slf4j.org/faq.html#logging_performance common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java (line 126) <https://reviews.apache.org/r/38465/#comment157727> Minor nit: use StringUtils.isNotBlank? common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java (line 129) <https://reviews.apache.org/r/38465/#comment157728> Instead of hard coding as / can we use new pth(parent, child) method? Just to make sure it works on all OS'es. common/src/main/java/org/apache/falcon/util/HdfsClassLoader.java (line 134) <https://reviews.apache.org/r/38465/#comment157729> can fileURL be null? common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java (line 76) <https://reviews.apache.org/r/38465/#comment157730> Instead of adding new type cant we reuse GENEARTE as import also generates feeds unless we save the operation type somewhere and can be useful to deintify the operation. falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HiveAssert.java (line 251) <https://reviews.apache.org/r/38465/#comment157731> I think this was not intended? oozie/src/main/java/org/apache/falcon/oozie/DatabaseImportWorkflowBuilder.java (line 57) <https://reviews.apache.org/r/38465/#comment157733> Have we tested this on secure cluster setup? For cases when workflow runs on diff m/c and hdfs is on diff node, do we have to pass any creds? oozie/src/main/java/org/apache/falcon/oozie/DatabaseImportWorkflowBuilder.java (line 147) <https://reviews.apache.org/r/38465/#comment157734> can exrtaArgs be null? oozie/src/main/java/org/apache/falcon/oozie/FeedImportCoordinatorBuilder.java (line 149) <https://reviews.apache.org/r/38465/#comment157735> Minor nit: use isNotBlank oozie/src/main/java/org/apache/falcon/oozie/FeedImportCoordinatorBuilder.java (line 162) <https://reviews.apache.org/r/38465/#comment157736> nit: use StringUtils.isEmptyNotBlank oozie/src/main/java/org/apache/falcon/oozie/ImportWorkflowBuilder.java (lines 62 - 66) <https://reviews.apache.org/r/38465/#comment157737> Please use the strings from WorkflowExecutionArgs instead of hardcoding oozie/src/main/java/org/apache/falcon/oozie/ImportWorkflowBuilder.java (line 63) <https://reviews.apache.org/r/38465/#comment157738> As import geanerate feed should this be set to Ignore? - Sowmya Ramesh 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 > >
