You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2016/01/11 11:02:07 UTC
Re: Review Request 40817: FALCON-1609: Add entity commands to falcon
spring-shell CLI
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40817/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 3:32 p.m.)
Review request for Falcon.
Bugs: FALCON-1609
https://issues.apache.org/jira/browse/FALCON-1609
Repository: falcon-git
Description
-------
putting for early review
Diffs (updated)
-----
cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java b7fa4cd524bfcd50ac260dadc2a1f94021b3df03
cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java dbd28fb74e2aaeedc94ed4ee43e688ea9efb15b0
cli/src/main/java/org/apache/falcon/cli/commands/FalconConnectionCommands.java cabe5a8083daf90456636d3378e410e3d95a4ada
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 6e091efcbe26ff7df7a3fe52e33b20f781b8d785
cli/src/main/java/org/apache/falcon/cli/commands/FalconInstanceCommands.java 8f3a2fc3ef57bb9026b98c85f51b363234d607c6
cli/src/test/java/org/apache/falcon/cli/commands/FalconCLITest.java PRE-CREATION
cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java PRE-CREATION
client/src/main/java/org/apache/falcon/cli/FalconCLI.java 24f230ae66ba710945be76d176fbdbdbbcb3721c
client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java cb39a2fe95beae32dee930afb9bb78d85ebb58ea
client/src/main/java/org/apache/falcon/client/FalconCLIException.java 51ef952474fda1242f4b4b5a06ccfd3011ea533a
client/src/main/java/org/apache/falcon/client/FalconClient.java b4b176224554fb5a86ebca2bcced980dc84d2dd6
client/src/main/java/org/apache/falcon/resource/EntityList.java f58169140af0f633463be93cff607c32e22254f4
Diff: https://reviews.apache.org/r/40817/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 40817: FALCON-1609: Add entity commands to falcon
spring-shell CLI
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Jan. 22, 2016, 1:50 p.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/resource/EntityList.java, line 55
> > <https://reviews.apache.org/r/40817/diff/3/?file=1192008#file1192008line55>
> >
> > This might affect other consumers of these fields. All of them might not show during compile time e.g. in regression code base they might be depending on these being static.
Enums are always static: http://stackoverflow.com/questions/23127926/static-enum-vs-non-static-enum
> On Jan. 22, 2016, 1:50 p.m., Ajay Yadava wrote:
> > cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java, line 47
> > <https://reviews.apache.org/r/40817/diff/3/?file=1192003#file1192003line47>
> >
> > no tests for Entity Commands?
Current Client has no tests, if I were to add tests, I'd be adding tests for the shell mocking the client.
> On Jan. 22, 2016, 1:50 p.m., Ajay Yadava wrote:
> > cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java, line 239
> > <https://reviews.apache.org/r/40817/diff/3/?file=1192000#file1192000line239>
> >
> > why is ENTITY_PREFIX required for validating orderBy?
Existing design, it's already required.
> On Jan. 22, 2016, 1:50 p.m., Ajay Yadava wrote:
> > cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java, line 81
> > <https://reviews.apache.org/r/40817/diff/3/?file=1192000#file1192000line81>
> >
> > 2 constants with same value, 1 can't be used in all places?
Not the same value. one is "entity", another is "entity "
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40817/#review115636
-----------------------------------------------------------
On Jan. 11, 2016, 5:02 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40817/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2016, 5:02 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1609
> https://issues.apache.org/jira/browse/FALCON-1609
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> putting for early review
>
>
> Diffs
> -----
>
> cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java b7fa4cd524bfcd50ac260dadc2a1f94021b3df03
> cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java dbd28fb74e2aaeedc94ed4ee43e688ea9efb15b0
> cli/src/main/java/org/apache/falcon/cli/commands/FalconConnectionCommands.java cabe5a8083daf90456636d3378e410e3d95a4ada
> cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 6e091efcbe26ff7df7a3fe52e33b20f781b8d785
> cli/src/main/java/org/apache/falcon/cli/commands/FalconInstanceCommands.java 8f3a2fc3ef57bb9026b98c85f51b363234d607c6
> cli/src/test/java/org/apache/falcon/cli/commands/FalconCLITest.java PRE-CREATION
> cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java PRE-CREATION
> client/src/main/java/org/apache/falcon/cli/FalconCLI.java 24f230ae66ba710945be76d176fbdbdbbcb3721c
> client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java cb39a2fe95beae32dee930afb9bb78d85ebb58ea
> client/src/main/java/org/apache/falcon/client/FalconCLIException.java 51ef952474fda1242f4b4b5a06ccfd3011ea533a
> client/src/main/java/org/apache/falcon/client/FalconClient.java b4b176224554fb5a86ebca2bcced980dc84d2dd6
> client/src/main/java/org/apache/falcon/resource/EntityList.java f58169140af0f633463be93cff607c32e22254f4
>
> Diff: https://reviews.apache.org/r/40817/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 40817: FALCON-1609: Add entity commands to falcon
spring-shell CLI
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40817/#review115636
-----------------------------------------------------------
cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java (line 44)
<https://reviews.apache.org/r/40817/#comment176623>
Just curious, why is it required?
cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java (line 33)
<https://reviews.apache.org/r/40817/#comment176624>
Hmm.. so our checkstyle allows wildcard imports for static members?
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 45)
<https://reviews.apache.org/r/40817/#comment176872>
required?
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 81)
<https://reviews.apache.org/r/40817/#comment176873>
2 constants with same value, 1 can't be used in all places?
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 86)
<https://reviews.apache.org/r/40817/#comment176630>
If I recall correctly not all of these parameters are mandatory.
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 109)
<https://reviews.apache.org/r/40817/#comment176861>
I think it should have been a String instead of File
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 117)
<https://reviews.apache.org/r/40817/#comment176862>
I believe this method should have been called update
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 206)
<https://reviews.apache.org/r/40817/#comment176866>
nit: should we rename ENTITY_PREFIX_SPACE to ENTITY_COMMAND_PREFIX or ENTITY_PREFIX? Will that be a misrepresentation?
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 239)
<https://reviews.apache.org/r/40817/#comment176874>
why is ENTITY_PREFIX required for validating orderBy?
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java (line 248)
<https://reviews.apache.org/r/40817/#comment176870>
I think we should be able to directly write it as final EntityType type instead of final String entityType.
If yes, then please make that change for all commands.
cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java (line 47)
<https://reviews.apache.org/r/40817/#comment176871>
no tests for Entity Commands?
client/src/main/java/org/apache/falcon/resource/EntityList.java (line 55)
<https://reviews.apache.org/r/40817/#comment176875>
This might affect other consumers of these fields. All of them might not show during compile time e.g. in regression code base they might be depending on these being static.
- Ajay Yadava
On Jan. 11, 2016, 11:32 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40817/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2016, 11:32 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1609
> https://issues.apache.org/jira/browse/FALCON-1609
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> putting for early review
>
>
> Diffs
> -----
>
> cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java b7fa4cd524bfcd50ac260dadc2a1f94021b3df03
> cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java dbd28fb74e2aaeedc94ed4ee43e688ea9efb15b0
> cli/src/main/java/org/apache/falcon/cli/commands/FalconConnectionCommands.java cabe5a8083daf90456636d3378e410e3d95a4ada
> cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 6e091efcbe26ff7df7a3fe52e33b20f781b8d785
> cli/src/main/java/org/apache/falcon/cli/commands/FalconInstanceCommands.java 8f3a2fc3ef57bb9026b98c85f51b363234d607c6
> cli/src/test/java/org/apache/falcon/cli/commands/FalconCLITest.java PRE-CREATION
> cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java PRE-CREATION
> client/src/main/java/org/apache/falcon/cli/FalconCLI.java 24f230ae66ba710945be76d176fbdbdbbcb3721c
> client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java cb39a2fe95beae32dee930afb9bb78d85ebb58ea
> client/src/main/java/org/apache/falcon/client/FalconCLIException.java 51ef952474fda1242f4b4b5a06ccfd3011ea533a
> client/src/main/java/org/apache/falcon/client/FalconClient.java b4b176224554fb5a86ebca2bcced980dc84d2dd6
> client/src/main/java/org/apache/falcon/resource/EntityList.java f58169140af0f633463be93cff607c32e22254f4
>
> Diff: https://reviews.apache.org/r/40817/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 40817: FALCON-1609: Add entity commands to falcon
spring-shell CLI
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40817/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 5:02 p.m.)
Review request for Falcon.
Bugs: FALCON-1609
https://issues.apache.org/jira/browse/FALCON-1609
Repository: falcon-git
Description
-------
putting for early review
Diffs (updated)
-----
cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java b7fa4cd524bfcd50ac260dadc2a1f94021b3df03
cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java dbd28fb74e2aaeedc94ed4ee43e688ea9efb15b0
cli/src/main/java/org/apache/falcon/cli/commands/FalconConnectionCommands.java cabe5a8083daf90456636d3378e410e3d95a4ada
cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 6e091efcbe26ff7df7a3fe52e33b20f781b8d785
cli/src/main/java/org/apache/falcon/cli/commands/FalconInstanceCommands.java 8f3a2fc3ef57bb9026b98c85f51b363234d607c6
cli/src/test/java/org/apache/falcon/cli/commands/FalconCLITest.java PRE-CREATION
cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java PRE-CREATION
client/src/main/java/org/apache/falcon/cli/FalconCLI.java 24f230ae66ba710945be76d176fbdbdbbcb3721c
client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java cb39a2fe95beae32dee930afb9bb78d85ebb58ea
client/src/main/java/org/apache/falcon/client/FalconCLIException.java 51ef952474fda1242f4b4b5a06ccfd3011ea533a
client/src/main/java/org/apache/falcon/client/FalconClient.java b4b176224554fb5a86ebca2bcced980dc84d2dd6
client/src/main/java/org/apache/falcon/resource/EntityList.java f58169140af0f633463be93cff607c32e22254f4
Diff: https://reviews.apache.org/r/40817/diff/
Testing
-------
Thanks,
Rajat Khandelwal