falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peeyush Bishnoi" <bpeey...@yahoo.co.in>
Subject Re: Review Request 40481: Need ability to provide the path for recipe files in command line
Date Mon, 23 Nov 2015 08:41:31 GMT


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> >

Thanks Ajay for reviewing.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 107
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line107>
> >
> >     I think only having a file exists check should be enough for validation.

This check I have put up, to ensure that recipe properties file name must match to recipe
name as per the recipe current design.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 43
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line43>
> >
> >     Do you think "file" instead of "properties" will be more intuitive?

"file" is more general but here for recipe we want to specifically provide recipe properties
file that's why "properties" looks better.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 110
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line110>
> >
> >     It is a mandatory parameter and if it is blank then it's fine?

recipePropertiesFile is mandatory parameter while passing through properties argument on command
line. Value to option "-properties" can't be null . If recipe properties file is not passed,
then properties file must be available inside "falcon.recipe.path" as usual.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 119
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line119>
> >
> >     Any reason for having the restriction that the properties file should match
the recipe name? I am not able to appreciate it's usefulness and it looks restrictive to me.

As per current design of recipe, it looks for template and properties file with recipe name
only.

String recipeFilePath = recipePath + File.separator + recipeName + TEMPLATE_SUFFIX;
String propertiesFilePath = recipePath + File.separator + recipeName + PROPERTIES_SUFFIX;

And through this patch, I don't want to change much behaviour, that's why I aligned with current
implementation of recipe.
Hoping such restriction will not be available when recipe server side will be implemented.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 1108
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131970#file1131970line1108>
> >
> >     Can you please add it in the client.properties so that it's easier for users
to figure out which property value to override?

done.


> On Nov. 20, 2015, 2:40 a.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 1152
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131970#file1131970line1152>
> >
> >     Earlier this path was the value of ""falcon.recipe.path" in client.properties
and now it has extra sub path added to it. Was it broken earlier?

No. Now templateFilePath is also from the value of "falcon.recipe.path" only. 
With this patch, if properties file is passed on command line then we will use properfiles
file directory path to look for recipe template and workflow file. This obtained directory
path from properties file will become "falcon.recipe.path".


- Peeyush


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


On Nov. 19, 2015, 2:02 p.m., Peeyush Bishnoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 2:02 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1588
>     https://issues.apache.org/jira/browse/FALCON-1588
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-1588 : Need ability to provide the path for recipe files in command line
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java b93fed2 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java c49dd08 
> 
> Diff: https://reviews.apache.org/r/40481/diff/
> 
> 
> Testing
> -------
> 
> yes.
> 
> 
> Thanks,
> 
> Peeyush Bishnoi
> 
>


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