falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Seetharam Venkatesh" <venkat...@innerzeal.com>
Subject Re: Review Request 26440: FALCON-753 Change the ownership for staging dir to user submitting the feed
Date Thu, 09 Oct 2014 23:27:54 GMT


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java, line
95
> > <https://reviews.apache.org/r/26440/diff/2/?file=716242#file716242line95>
> >
> >     Let us call this as getFileSystemAsEntityOwner

Makes sense


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 166
> > <https://reviews.apache.org/r/26440/diff/2/?file=716243#file716243line166>
> >
> >     Bunch of helper set Functions have been removed. Are they not required any more?

They were unused. Thought I'd clean up. But if you have concerns, can revert back.


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 201
> > <https://reviews.apache.org/r/26440/diff/2/?file=716243#file716243line201>
> >
> >     Perhaps 1 is the default. We should discuss this with @Shaik

OK.


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 285
> > <https://reviews.apache.org/r/26440/diff/2/?file=716243#file716243line285>
> >
> >     Doesn't seem to be using a variable for a magicConstant field. Why is this being
suppressed?

It was annoying to see many warnings with Frequency, specifically frequency.getTimeUnit().getCalendarUnit()


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 462
> > <https://reviews.apache.org/r/26440/diff/2/?file=716243#file716243line462>
> >
> >     No new forced casting introduced. Is this required?

Existing Unchecked cast: 'org.apache.falcon.entity.v0.feed.Feed' to 'T'


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java, line
126
> > <https://reviews.apache.org/r/26440/diff/2/?file=716244#file716244line126>
> >
> >     this is note earlier about HADOOP-10215. Is this resolved in 2.5  and hence
removed ?

Yes, the jira is now resolved.


> On Oct. 9, 2014, 1:38 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java, line
300
> > <https://reviews.apache.org/r/26440/diff/2/?file=716244#file716244line300>
> >
> >     Can we refactor this to send the path and expected permission mask, instead
of conditionally checking for working & staging inside this ?

ok.


- Seetharam


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


On Oct. 9, 2014, 12:42 a.m., Srikanth Sundarrajan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26440/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2014, 12:42 a.m.)
> 
> 
> Review request for Falcon and Seetharam Venkatesh.
> 
> 
> Bugs: FALCON-753
>     https://issues.apache.org/jira/browse/FALCON-753
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-753 Change the ownership for staging dir to user submitting the feed
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java c315c25

>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java b8f2d7d 
>   common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java fbbdbcb

>   common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java 8a3f669 
>   common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java bbb2b12

>   common/src/main/java/org/apache/falcon/hadoop/HadoopClientFactory.java ecdbf14 
>   common/src/main/java/org/apache/falcon/security/CurrentUser.java b7d2c66 
>   common/src/main/java/org/apache/falcon/security/DefaultAuthorizationProvider.java e90518d

>   common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java f3e643e

>   common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java 6ad742e 
>   common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 2d41661 
>   common/src/test/java/org/apache/falcon/hadoop/HadoopClientFactoryTest.java 8f25f57

>   common/src/test/java/org/apache/falcon/security/CurrentUserTest.java a1861e1 
>   docs/src/site/twiki/EntitySpecification.twiki a710dfd 
>   docs/src/site/twiki/InstallationSteps.twiki 0e3bfcf 
>   docs/src/site/twiki/OnBoarding.twiki 75d0d7e 
>   docs/src/site/twiki/Security.twiki 2e97c8b 
>   hadoop-dependencies/src/main/java/org/apache/falcon/hadoop/JailedFileSystem.java 72e390e

>   messaging/src/main/java/org/apache/falcon/messaging/JMSMessageConsumer.java 4a0bc2a

>   messaging/src/main/java/org/apache/falcon/messaging/JMSMessageProducer.java 629e6a5

>   oozie/src/main/java/org/apache/falcon/logging/JobLogMover.java 1a08ada 
>   oozie/src/main/java/org/apache/falcon/logging/LogProvider.java 4ed8f52 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java 82f7251 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieCoordinatorBuilder.java fe2136b 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieEntityBuilder.java 1c3085c 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java
2339284 
>   oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationCoordinatorBuilder.java
801d733 
>   oozie/src/main/java/org/apache/falcon/oozie/process/ProcessBundleBuilder.java ab38259

>   oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionWorkflowBuilder.java
865beaf 
>   oozie/src/main/java/org/apache/falcon/service/SharedLibraryHostingService.java d273d61

>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieHouseKeepingService.java
bbed949 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java fd8c63f

>   oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java
68911ce 
>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java 41cd601 
>   prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java b61360b

>   replication/src/main/java/org/apache/falcon/replication/FeedReplicator.java d927fff

>   rerun/src/main/java/org/apache/falcon/latedata/LateDataHandler.java d854bdd 
>   rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 459da84

>   rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java 80a3b83

>   rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java bacc20f 
>   retention/src/main/java/org/apache/falcon/retention/FeedEvictor.java f7a4493 
>   test-util/src/main/java/org/apache/falcon/cluster/util/EmbeddedCluster.java b1e518d

>   webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 0943103 
>   webapp/src/test/java/org/apache/falcon/late/LateDataHandlerIT.java 96c99c5 
>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java ed70a0b

>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java e9545d1 
>   webapp/src/test/java/org/apache/falcon/validation/ClusterEntityValidationIT.java 6a65bf6

>   webapp/src/test/resources/cluster-template.xml bc17fb9 
>   webapp/src/test/resources/process-template.xml 0add06a 
> 
> Diff: https://reviews.apache.org/r/26440/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>


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