You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Peeyush Bishnoi <bp...@yahoo.co.in> on 2015/11/19 15:02:31 UTC
Review Request 40481: Need ability to provide the path for recipe
files in command line
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/
-----------------------------------------------------------
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
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107597
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Nov. 23, 2015, 8:43 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 8:43 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107713
-----------------------------------------------------------
Ship it!
Ship It!
- sandeep samudrala
On Nov. 23, 2015, 8:43 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 8:43 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107767
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Nov. 24, 2015, 10:31 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 24, 2015, 10:31 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/
-----------------------------------------------------------
(Updated Nov. 24, 2015, 10:31 a.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 (updated)
-----
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java b93fed2
client/src/main/java/org/apache/falcon/client/FalconClient.java c49dd08
src/conf/client.properties 7925009
Diff: https://reviews.apache.org/r/40481/diff/
Testing
-------
yes.
Thanks,
Peeyush Bishnoi
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Nov. 24, 2015, 5:33 a.m., Sowmya Ramesh wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 24
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line24>
> >
> > Minor nit: use import org.apache.commons.lang3.StringUtils;
done.
> On Nov. 24, 2015, 5:33 a.m., Sowmya Ramesh wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java, line 109
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131969#file1131969line109>
> >
> >
fixed.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107222
-----------------------------------------------------------
On Nov. 23, 2015, 8:43 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 8:43 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107222
-----------------------------------------------------------
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 24)
<https://reviews.apache.org/r/40481/#comment166240>
Minor nit: use import org.apache.commons.lang3.StringUtils;
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 109)
<https://reviews.apache.org/r/40481/#comment166243>
- Sowmya Ramesh
On Nov. 23, 2015, 8:43 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 8:43 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107712
-----------------------------------------------------------
Ship it!
Ship It!
- Balu Vellanki
On Nov. 23, 2015, 8:43 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40481/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 8:43 a.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
> src/conf/client.properties 7925009
>
> Diff: https://reviews.apache.org/r/40481/diff/
>
>
> Testing
> -------
>
> yes.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/
-----------------------------------------------------------
(Updated Nov. 23, 2015, 8:43 a.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 (updated)
-----
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java b93fed2
client/src/main/java/org/apache/falcon/client/FalconClient.java c49dd08
src/conf/client.properties 7925009
Diff: https://reviews.apache.org/r/40481/diff/
Testing
-------
yes.
Thanks,
Peeyush Bishnoi
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107262
-----------------------------------------------------------
Apart from minor suggestion, code looks ok to me.
- Balu Vellanki
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
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> 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
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107292
-----------------------------------------------------------
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 43)
<https://reviews.apache.org/r/40481/#comment166323>
Do you think "file" instead of "properties" will be more intuitive?
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 107)
<https://reviews.apache.org/r/40481/#comment166327>
I think only having a file exists check should be enough for validation.
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 110)
<https://reviews.apache.org/r/40481/#comment166324>
It is a mandatory parameter and if it is blank then it's fine?
client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java (line 119)
<https://reviews.apache.org/r/40481/#comment166325>
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.
client/src/main/java/org/apache/falcon/client/FalconClient.java (line 1108)
<https://reviews.apache.org/r/40481/#comment166326>
Can you please add it in the client.properties so that it's easier for users to figure out which property value to override?
client/src/main/java/org/apache/falcon/client/FalconClient.java (line 1152)
<https://reviews.apache.org/r/40481/#comment166329>
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?
- Ajay Yadava
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
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Nov. 19, 2015, 10:48 p.m., Balu Vellanki wrote:
> >
Thanks Balu for reviewing.
> On Nov. 19, 2015, 10:48 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 1123
> > <https://reviews.apache.org/r/40481/diff/1/?file=1131970#file1131970line1123>
> >
> > An error message explaining where this property should be set will be useful. This message seems confusing to the user.
fixed.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107261
-----------------------------------------------------------
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
>
>
Re: Review Request 40481: Need ability to provide the path for recipe
files in command line
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40481/#review107261
-----------------------------------------------------------
client/src/main/java/org/apache/falcon/client/FalconClient.java (line 1123)
<https://reviews.apache.org/r/40481/#comment166292>
An error message explaining where this property should be set will be useful. This message seems confusing to the user.
- Balu Vellanki
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
>
>