You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by samarth gupta <sa...@gmail.com> on 2014/10/28 14:14:56 UTC
Review Request 27280: https://issues.apache.org/jira/browse/FALCON-618
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/
-----------------------------------------------------------
Review request for Falcon.
Repository: falcon-git
Description
-------
https://issues.apache.org/jira/browse/FALCON-618
Diffs
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
Diff: https://reviews.apache.org/r/27280/diff/
Testing
-------
Thanks,
samarth gupta
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by Ruslan Ostafiychuk <ro...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review59148
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100469>
checkstyle:
should be 'LOGGER'
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100466>
checkstyle:
Variable ... must be private ...
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100467>
checkstyle:
'private' modifier out of order with the JLS suggestions.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100464>
do you use our checkstyle config from checkstyle subproject? I have this error found by checkstyle:
'(' should be on the previous line. (222:17)
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100465>
checkstyle:
'(' should be on the previous line. (233:29)
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100470>
checkstyle:
whitespace expected before '//'
- Ruslan Ostafiychuk
On Oct. 28, 2014, 1:15 p.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 1:15 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review59260
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java
<https://reviews.apache.org/r/27280/#comment100531>
This should probably be called as REAL_USER ?
https://hadoop.apache.org/docs/r1.0.4/api/org/apache/hadoop/security/UserGroupInformation.html#createProxyUser(java.lang.String, org.apache.hadoop.security.UserGroupInformation)
- Raghav Gautam
On Oct. 30, 2014, 5:46 a.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 5:46 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 5ee6dd2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 73cf8c8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java 991732b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by samarth gupta <sa...@gmail.com>.
> On Oct. 30, 2014, 8:50 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java, line 493
> > <https://reviews.apache.org/r/27280/diff/2/?file=742868#file742868line493>
> >
> > Are we expecting widespread usage of these methods ? If not can we make it local to the test.
yes , i expect widespred use of the method in future, particularly with introduction of ACL in cluster and process also. We can move to local test for now, but problem with that is many times we forget abt it and result in rewriting the same helper methods.
> On Oct. 30, 2014, 8:50 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java, line 499
> > <https://reviews.apache.org/r/27280/diff/2/?file=742868#file742868line499>
> >
> > Are we assuming that the tests are being run as oozie ?
no we are not. We are just trying to send request as OOZIE user. What i am trying to achive is since we need to change / create path permission , it should be done as one of the users who is authorized to proxy on behalf of hdfs ( on a secutiry enabled hdfs where we dont have "chmod 777 /" ) . only assumtion is that since our regression setup requires oozie running , user who started oozie must be present as proxy in hdfs conf.
> On Oct. 30, 2014, 8:50 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 496
> > <https://reviews.apache.org/r/27280/diff/2/?file=742871#file742871line496>
> >
> > Can we use createMissingDependenciesForBundle() method here ? To me it seems that they are doing similar things. Is a unification possible ?
i tried that initially. But problem i faced was, different namenode for hdfs and conf object. For replication coord is created on target but the missing dependency is of source cluster which is having different end point of "fs.default.name" . So hdfs client was giving error while trying to create it. Will have a relook at it.
- samarth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review59247
-----------------------------------------------------------
On Oct. 30, 2014, 12:46 p.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 12:46 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 5ee6dd2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 73cf8c8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java 991732b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by Ruslan Ostafiychuk <ro...@hortonworks.com>.
> On Oct. 30, 2014, 8:50 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java, line 146
> > <https://reviews.apache.org/r/27280/diff/2/?file=742877#file742877line146>
> >
> > I thing line exceeds 100 char limit ?
We have 120 char limit in checkstyle.xml:
<module name="LineLength">
<property name="max" value="120"/>
</module>
- Ruslan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review59247
-----------------------------------------------------------
On Oct. 30, 2014, 12:46 p.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 12:46 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 5ee6dd2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 73cf8c8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java 991732b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review59247
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java
<https://reviews.apache.org/r/27280/#comment100517>
Are we expecting widespread usage of these methods ? If not can we make it local to the test.
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java
<https://reviews.apache.org/r/27280/#comment100516>
Are we assuming that the tests are being run as oozie ?
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java
<https://reviews.apache.org/r/27280/#comment100519>
Can we use createMissingDependenciesForBundle() method here ? To me it seems that they are doing similar things. Is a unification possible ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100523>
mass import ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java
<https://reviews.apache.org/r/27280/#comment100520>
I thing line exceeds 100 char limit ?
- Raghav Gautam
On Oct. 30, 2014, 5:46 a.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 5:46 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 5ee6dd2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 73cf8c8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java 991732b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by samarth gupta <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/
-----------------------------------------------------------
(Updated Oct. 30, 2014, 12:46 p.m.)
Review request for Falcon.
Repository: falcon-git
Description
-------
https://issues.apache.org/jira/browse/FALCON-618
Diffs (updated)
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 5ee6dd2
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 73cf8c8
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/EntityDryRunTest.java 991732b
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
Diff: https://reviews.apache.org/r/27280/diff/
Testing
-------
Thanks,
samarth gupta
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by samarth gupta <sa...@gmail.com>.
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java, line 198
> > <https://reviews.apache.org/r/27280/diff/1/?file=735308#file735308line198>
> >
> > This method seems scenario spcific. May be it should be part of feedRetryTest().
> >
> > Inline perhaps ?
i though this can be used in other test also. will move to RetryTest for now, if in future need arise can move to common util. leaving it seperate and not making inline.
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java, line 498
> > <https://reviews.apache.org/r/27280/diff/1/?file=735310#file735310line498>
> >
> > This assumes that test user has privileges to proxy as hdfs. Which is a strong assumption.
> >
> > ACL_OWNER for intents and purposes is current user. Perhaps it would be better to setPermission as current user ?
> >
> > Also, if directory creation and setting permissions is not being done frequently, can we keep them as two separate methods.
in the current code variable PROXY_USER defined at top as private static final String PROXY_USER = "oozie" ; is trying to act as hdfs.
Here assumtion is that user "oozie" has privileges on hdfs.
it was done assuming oozie is running as user oozie and will be present in hadoop conf where regression is running.
We surely make it configurable if required.
permissions are being set as ACL_OWNER
authFs.setPermission(path, fsPermission);
authFs.setOwner(path, MerlinConstants.ACL_OWNER, PROXY_GROUP);
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 127
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line127>
> >
> > changing name of setRetry to setProcessRetry would make things much clearer.
done
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 285
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line285>
> >
> > javadoc ?
done
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 289
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line289>
> >
> > unit of delayVal ?
since this is a test case specific method where we know the limits of delay , i dont think we shuold face any overflow. If that was your concern
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 295
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line295>
> >
> > may be use switch instead of if ?
since only one "if else" is present, i dont think it will make much difference.
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 297
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line297>
> >
> > Can you please simplify this expression - may be by using temp variables ?
i have added java doc for the same. that should help.
> On Oct. 28, 2014, 10:06 p.m., Raghav Gautam wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java, line 221
> > <https://reviews.apache.org/r/27280/diff/1/?file=735317#file735317line221>
> >
> > formatting ?
i am not getting any new checkstyle errors. Should i be checking something more ?
- samarth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review58882
-----------------------------------------------------------
On Oct. 28, 2014, 1:15 p.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 1:15 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/#review58882
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java
<https://reviews.apache.org/r/27280/#comment100027>
This method seems scenario spcific. May be it should be part of feedRetryTest().
Inline perhaps ?
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java
<https://reviews.apache.org/r/27280/#comment100028>
This assumes that test user has privileges to proxy as hdfs. Which is a strong assumption.
ACL_OWNER for intents and purposes is current user. Perhaps it would be better to setPermission as current user ?
Also, if directory creation and setting permissions is not being done frequently, can we keep them as two separate methods.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100031>
changing name of setRetry to setProcessRetry would make things much clearer.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100052>
formatting ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100051>
formatting ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100044>
javadoc ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100047>
unit of delayVal ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100050>
may be use switch instead of if ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100049>
may be use switch instead of if ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java
<https://reviews.apache.org/r/27280/#comment100048>
Can you please simplify this expression - may be by using temp variables ?
- Raghav Gautam
On Oct. 28, 2014, 6:15 a.m., samarth gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27280/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 6:15 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/FALCON-618
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
> falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
>
> Diff: https://reviews.apache.org/r/27280/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> samarth gupta
>
>
Re: Review Request 27280:
https://issues.apache.org/jira/browse/FALCON-618
Posted by samarth gupta <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27280/
-----------------------------------------------------------
(Updated Oct. 28, 2014, 1:15 p.m.)
Review request for Falcon.
Repository: falcon-git
Description
-------
https://issues.apache.org/jira/browse/FALCON-618
Diffs
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java 02f572e
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/HadoopUtil.java 024a652
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java c6217c1
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java d4f31e9
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java f564bb4
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java a94cf1a
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java 3f7258e
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/RetryTest.java PRE-CREATION
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java 9b5d770
falcon-regression/merlin/src/test/resources/RetryTests/valid1/bundle1/process-agg.xml 03cb727
Diff: https://reviews.apache.org/r/27280/diff/
Testing
-------
Thanks,
samarth gupta