falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pallavi Rao" <pallavi....@inmobi.com>
Subject Re: Review Request 38294: FALCON-1434 Enhance schedule API to accept key-value properties
Date Mon, 14 Sep 2015 06:22:06 GMT


> On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 904
> > <https://reviews.apache.org/r/38294/diff/1/?file=1068154#file1068154line904>
> >
> >     Do you not allow ":" character in the value?  I see value in allowing ":" in
the value for properties. Splitting kvPair into two Strings might be more helpful. 
> >     
> >     String[] keyValue = kvPair.trim().split(":", 2);

Makes sense. Will allow : in values.


> On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 905
> > <https://reviews.apache.org/r/38294/diff/1/?file=1068154#file1068154line905>
> >
> >     This can allow the keyValue[0] to be empty. I think we should not have empty
property keys.

Good catch. Added an additional check.


> On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote:
> > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java, line 337
> > <https://reviews.apache.org/r/38294/diff/1/?file=1068156#file1068156line337>
> >
> >     Please add " :value1" to list of invalid props.

Done.


> On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 94
> > <https://reviews.apache.org/r/38294/diff/1/?file=1068151#file1068151line94>
> >
> >     Minor nit - Can we use "properties" instead?
> >     
> >     Can we also add some tests to EntityManagerJerseyIT?

Changed to "properties".

Just enhanced the existing tests in FalconCLIIT and EntityManagerJerseyIT. Didn't add any
new test as the properties aren't really used yet.


- Pallavi


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


On Sept. 14, 2015, 6:21 a.m., Pallavi Rao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38294/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2015, 6:21 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1434
>     https://issues.apache.org/jira/browse/FALCON-1434
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> The schedule API will be enhanced to accept a key-value properties. This is a foundation
to enable users to specify the scheduler on which they want to schedule the entity. This in
turn enables migration to native scheduler from Oozie.
> Example:
> bin/falcon entity -schedule -props falcon.scheduler=native -name <entity name>
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java d4da302 
>   client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 282b41b 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 44436d2 
>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java 25d9008 
>   common/src/main/java/org/apache/falcon/workflow/engine/AbstractWorkflowEngine.java
ea86c2a 
>   common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java cfdc84d 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java 5f79ca1

>   prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
f9405dc 
>   prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java
ceabb06 
>   unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java eb65cb3 
>   unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java 997b301 
>   unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 498f50e 
>   webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java 1f8cc1b

>   webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 0062070 
>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java bcd3bd5

>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java 54671fb 
> 
> Diff: https://reviews.apache.org/r/38294/diff/
> 
> 
> Testing
> -------
> 
> UT added
> Manually tested to ensure CLI accepts properties and it is propagated.
> 
> 
> Thanks,
> 
> Pallavi Rao
> 
>


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