You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by "Shwetha G S (Created) (JIRA)" <ji...@apache.org> on 2012/02/03 08:01:54 UTC
[jira] [Created] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
checkMultipleTimeInstances doesn't work for EL extensions
---------------------------------------------------------
Key: OOZIE-675
URL: https://issues.apache.org/jira/browse/OOZIE-675
Project: Oozie
Issue Type: Bug
Reporter: Shwetha G S
To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "Srikanth Sundarrajan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Srikanth Sundarrajan updated OOZIE-675:
---------------------------------------
Component/s: core
Affects Version/s: 3.2.0
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "Shwetha G S (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203302#comment-13203302 ]
Shwetha G S commented on OOZIE-675:
-----------------------------------
Yes, coord:formatTime is not valid in instance tag. Also, it can't be used for today as today also supports parameters of hr and min which are added to start day. For example, today(2,0) is 2nd hour of today.
Also, with EL extensions, users can define expressions in any way. Checking for ',' doesn't looks like the right way to check for multiple instances.
This issue wasn't there in 3.0.2. We are planning to use the new release 3.1.3. Can you please fix this in 3.1.3
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225083#comment-13225083 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/
-----------------------------------------------------------
(Updated 2012-03-08 08:19:24.114428)
Review request for oozie.
Summary (updated)
-------
checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
This addresses bug OOZIE-675.
https://issues.apache.org/jira/browse/OOZIE-675
Diffs
-----
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
Diff: https://reviews.apache.org/r/4222/diff
Testing
-------
New test case added to TestCoordSubmitXCommand.java for verifying the changes
Thanks,
Srikanth
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225472#comment-13225472 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5747
-----------------------------------------------------------
There is another type of EL expression of the format e.g. ${coord:future(start_instance,max_instance)} that needs to be tested. This type carries a "comma" within the function.
- Mona
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13226008#comment-13226008 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 20:34:17, Mona Chitnis wrote:
bq. > There is another type of EL expression of the format e.g. ${coord:future(start_instance,max_instance)} that needs to be tested. This type carries a "comma" within the function.
bq.
bq. Alejandro Abdelnur wrote:
bq. Srikanth, looks good. Would be possible to have a negative test as well?
I have modified input for testBasicSubmitWithMultipleInstancesOutputEvent, to include tests for future() function as well.
testBasicSubmitWithMultipleInstancesInputEvent::case 1 & testBasicSubmitWithMultipleInstancesOutputEvent:case 1 are actually a negative case where "," is present outside a function, which are gated appropriately. testBasicSubmitWithMultipleInstancesOutputEvent::Case 4 checks for both input and output with "," within the context of a function.
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5747
-----------------------------------------------------------
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13229488#comment-13229488 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5956
-----------------------------------------------------------
Ship it!
Ship it after 2 minor suggestions. I've tested the patch locally.
trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java
<https://reviews.apache.org/r/4222/#comment12927>
Should be CASE 4
trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java
<https://reviews.apache.org/r/4222/#comment12926>
minor space
- Mona
On 2012-03-09 11:06:47, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-09 11:06:47)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-input-instance4.xml PRE-CREATION
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13226009#comment-13226009 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 23:06:09, Mona Chitnis wrote:
bq. > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38
bq. > <https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38>
bq. >
bq. > Aren't these 2 instances?
bq.
bq. Mona Chitnis wrote:
bq. Also, can a test be added for coord:dateOffset('String strBaseDate', 'int offset', 'String unit')? Thanks
Included additional scenarios for testing this. Input coord-multiple-input-instance4.xml
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5757
-----------------------------------------------------------
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225644#comment-13225644 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 20:34:17, Mona Chitnis wrote:
bq. > There is another type of EL expression of the format e.g. ${coord:future(start_instance,max_instance)} that needs to be tested. This type carries a "comma" within the function.
Srikanth, looks good. Would be possible to have a negative test as well?
- Alejandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5747
-----------------------------------------------------------
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225096#comment-13225096 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5707
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java
<https://reviews.apache.org/r/4222/#comment12443>
Line 323-330 and line 333-340 look similar.
is it possible to create a method and invoke it from the same place.
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java
<https://reviews.apache.org/r/4222/#comment12445>
how to identify this ${func(), func2()}?
that means in one ${ } there are multiple functions.
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java
<https://reviews.apache.org/r/4222/#comment12444>
is it possible to add one/two unit test case for this new method in TestELEvaluator.jav? Both cases +ve and -ve.
This is a critical pieces, hence asking to be more cautious.
- Mohammad
On 2012-03-08 08:19:24, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 08:19:24)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225248#comment-13225248 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/
-----------------------------------------------------------
(Updated 2012-03-08 15:08:40.695265)
Review request for oozie.
Summary
-------
checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
This addresses bug OOZIE-675.
https://issues.apache.org/jira/browse/OOZIE-675
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
Diff: https://reviews.apache.org/r/4222/diff
Testing (updated)
-------
* New test case added to TestCoordSubmitXCommand.java for verifying the changes
* Added additional test cases to TestELEvaluator.java as well
Thanks,
Srikanth
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Assigned] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "Alejandro Abdelnur (Assigned) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alejandro Abdelnur reassigned OOZIE-675:
----------------------------------------
Assignee: Srikanth Sundarrajan
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Assignee: Srikanth Sundarrajan
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13226010#comment-13226010 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/
-----------------------------------------------------------
(Updated 2012-03-09 11:06:47.528914)
Review request for oozie.
Changes
-------
New patch uploaded after incorporating review comments. Thanks for taking time to review.
Summary
-------
checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
This addresses bug OOZIE-675.
https://issues.apache.org/jira/browse/OOZIE-675
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
trunk/core/src/test/resources/coord-multiple-input-instance4.xml PRE-CREATION
trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
Diff: https://reviews.apache.org/r/4222/diff
Testing
-------
* New test case added to TestCoordSubmitXCommand.java for verifying the changes
* Added additional test cases to TestELEvaluator.java as well
Thanks,
Srikanth
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13224360#comment-13224360 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/
-----------------------------------------------------------
Review request for oozie.
Summary
-------
checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
This addresses bug OOZIE-675.
https://issues.apache.org/jira/browse/OOZIE-675
Diffs
-----
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
Diff: https://reviews.apache.org/r/4222/diff
Testing
-------
New test case added to TestCoordSubmitXCommand.java for verifying the changes
Thanks,
Srikanth
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225261#comment-13225261 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 09:01:30, Mohammad Islam wrote:
bq. > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323
bq. > <https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323>
bq. >
bq. > Line 323-330 and line 333-340 look similar.
bq. > is it possible to create a method and invoke it from the same place.
bq.
bq. Srikanth Sundarrajan wrote:
bq. Yes. Makes sense. Will put up a revised patch shortly
On closer inspection turns out that the correctAction hints and the error messages are different for the scenarios. Have refactored a bit to move the exception handling part for both cases outside of the main method and made it a bit modular.
bq. On 2012-03-08 09:01:30, Mohammad Islam wrote:
bq. > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221
bq. > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221>
bq. >
bq. > how to identify this ${func(), func2()}?
bq. > that means in one ${ } there are multiple functions.
bq.
bq. Srikanth Sundarrajan wrote:
bq. I guess this wont be a valid el expression. I am assuming ExpressionEvaluator would reject it. Will check and confirm.
Added tests in TestELEvaluator to test this case.
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5707
-----------------------------------------------------------
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225650#comment-13225650 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5757
-----------------------------------------------------------
trunk/core/src/test/resources/coord-multiple-output-instance4.xml
<https://reviews.apache.org/r/4222/#comment12544>
Aren't these 2 instances?
- Mona
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "Shwetha G S (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13199605#comment-13199605 ]
Shwetha G S commented on OOZIE-675:
-----------------------------------
Yes, your understanding of the issue is right.
today(0,0) points to start of today(00:00 hours). This can't be done by date formatting. we use today(0,0) to specify start instance of a dataset(instead of coord:current)
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225157#comment-13225157 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 09:01:30, Mohammad Islam wrote:
bq. > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221
bq. > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221>
bq. >
bq. > how to identify this ${func(), func2()}?
bq. > that means in one ${ } there are multiple functions.
I guess this wont be a valid el expression. I am assuming ExpressionEvaluator would reject it. Will check and confirm.
bq. On 2012-03-08 09:01:30, Mohammad Islam wrote:
bq. > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 229
bq. > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line229>
bq. >
bq. > is it possible to add one/two unit test case for this new method in TestELEvaluator.jav? Both cases +ve and -ve.
bq. > This is a critical pieces, hence asking to be more cautious.
Sure. Will put up an updated patch
bq. On 2012-03-08 09:01:30, Mohammad Islam wrote:
bq. > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323
bq. > <https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323>
bq. >
bq. > Line 323-330 and line 333-340 look similar.
bq. > is it possible to create a method and invoke it from the same place.
Yes. Makes sense. Will put up a revised patch shortly
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5707
-----------------------------------------------------------
On 2012-03-08 08:19:24, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 08:19:24)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225696#comment-13225696 ]
jiraposter@reviews.apache.org commented on OOZIE-675:
-----------------------------------------------------
bq. On 2012-03-08 23:06:09, Mona Chitnis wrote:
bq. > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38
bq. > <https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38>
bq. >
bq. > Aren't these 2 instances?
Also, can a test be added for coord:dateOffset('String strBaseDate', 'int offset', 'String unit')? Thanks
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4222/#review5757
-----------------------------------------------------------
On 2012-03-08 15:08:40, Srikanth Sundarrajan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4222/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 15:08:40)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. checkMultipleTimeInstances doesn't work for EL extensions. EX: ${coord:formatTime(coord:current(0),'yyyy-MM-dd')}
bq.
bq.
bq. This addresses bug OOZIE-675.
bq. https://issues.apache.org/jira/browse/OOZIE-675
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 1297367
bq. trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 1297367
bq. trunk/core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 1297367
bq. trunk/core/src/test/resources/coord-multiple-output-instance4.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4222/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * New test case added to TestCoordSubmitXCommand.java for verifying the changes
bq. * Added additional test cases to TestELEvaluator.java as well
bq.
bq.
bq. Thanks,
bq.
bq. Srikanth
bq.
bq.
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.0
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OOZIE-675) checkMultipleTimeInstances doesn't
work for EL extensions
Posted by "Mohammad Kamrul Islam (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OOZIE-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13199599#comment-13199599 ]
Mohammad Kamrul Islam commented on OOZIE-675:
---------------------------------------------
Are you talking about "if (instanceValue.contains(",")) {" part of the method?
As of now, we expect built-in EL functions like current() and latest() or specific dates. None of those has "," and therefore it is not an issue.
However , your EL extension has multiple parameters and hence a comma (," is there that breaks throw the exception.
Did I understand the issue correctly?
Probable resolution:
We want to make sure there is only one ultimate value per instance. We could do this checking/enforcement after the EL resolution stage where we can ensure if there are indeed multiple values or not. In general, this EL evaluation needs to be done later (not during job submission). In this resolution, job submission will be fine. However, the error will come very late stage and fail.
In another note:
By your short description, I think if you could instead consider to use the built-in two EL functions for your today(0, 0).
coord:formatTime(coord.nominalTime(), "The required Date Format")
One thing, I'm not sure those functions are valid in instance tag. If not , we could allow those in that context.
Ref: http://yahoo.github.com/oozie/releases/3.1.0/CoordinatorFunctionalSpec.html#a6.8.2._coord:formatTimeString_ts_String_format_EL_Function_since_Oozie_2.3.2
> checkMultipleTimeInstances doesn't work for EL extensions
> ---------------------------------------------------------
>
> Key: OOZIE-675
> URL: https://issues.apache.org/jira/browse/OOZIE-675
> Project: Oozie
> Issue Type: Bug
> Reporter: Shwetha G S
> Labels: EL, extensions
>
> To guard against specifying multiple instances in instance tag, CoordSubmitXCommand.checkMultipleTimeInstances checks for ',' in instance tag. This breaks for EL extensions. We have EL extension today(0,0) which maps to start day of nominal time. today(0,0) resolves to a single instance and presence of ',' doesn't imply that it resolves to multiple instances
> Can you please fix this
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira