Oozie
  1. Oozie
  2. OOZIE-675

checkMultipleTimeInstances doesn't work for EL extensions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.0
    • Fix Version/s: 3.2.0
    • Component/s: core
    • Labels:

      Description

      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

        Activity

        Hide
        Mohammad Kamrul Islam added a comment -

        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

        Show
        Mohammad Kamrul Islam added a comment - 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
        Hide
        Shwetha G S added a comment -

        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)

        Show
        Shwetha G S added a comment - 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)
        Hide
        Shwetha G S added a comment -

        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

        Show
        Shwetha G S added a comment - 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Show
        Srikanth Sundarrajan added a comment - https://reviews.apache.org/r/4222/
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 08:19:24)

        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 08:19:24) 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 09:01:30, Mohammad Islam wrote:

        > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221

        > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221>

        >

        > how to identify this ${func(), func2()}?

        > 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.

        On 2012-03-08 09:01:30, Mohammad Islam wrote:

        > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 229

        > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line229>

        >

        > 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.

        Sure. Will put up an updated patch

        On 2012-03-08 09:01:30, Mohammad Islam wrote:

        > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323

        > <https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323>

        >

        > Line 323-330 and line 333-340 look similar.

        > 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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 08:19:24)

        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

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 09:01:30, Mohammad Islam wrote: > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221 > < https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221 > > > how to identify this ${func(), func2()}? > 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. On 2012-03-08 09:01:30, Mohammad Islam wrote: > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 229 > < https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line229 > > > 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. Sure. Will put up an updated patch On 2012-03-08 09:01:30, Mohammad Islam wrote: > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323 > < https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323 > > > Line 323-330 and line 333-340 look similar. > 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 08:19:24) 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 09:01:30, Mohammad Islam wrote:

        > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323

        > <https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323>

        >

        > Line 323-330 and line 333-340 look similar.

        > is it possible to create a method and invoke it from the same place.

        Srikanth Sundarrajan wrote:

        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.

        On 2012-03-08 09:01:30, Mohammad Islam wrote:

        > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221

        > <https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221>

        >

        > how to identify this ${func(), func2()}?

        > that means in one ${ } there are multiple functions.

        Srikanth Sundarrajan wrote:

        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 09:01:30, Mohammad Islam wrote: > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 323 > < https://reviews.apache.org/r/4222/diff/1/?file=88746#file88746line323 > > > Line 323-330 and line 333-340 look similar. > is it possible to create a method and invoke it from the same place. Srikanth Sundarrajan wrote: 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. On 2012-03-08 09:01:30, Mohammad Islam wrote: > trunk/core/src/main/java/org/apache/oozie/util/ELEvaluator.java, line 221 > < https://reviews.apache.org/r/4222/diff/1/?file=88747#file88747line221 > > > how to identify this ${func(), func2()}? > that means in one ${ } there are multiple functions. Srikanth Sundarrajan wrote: 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 20:34:17, Mona Chitnis wrote:

        > 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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 20:34:17, Mona Chitnis wrote: > 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 23:06:09, Mona Chitnis wrote:

        > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38

        > <https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38>

        >

        > 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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 23:06:09, Mona Chitnis wrote: > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38 > < https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38 > > > 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 20:34:17, Mona Chitnis wrote:

        > 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.

        Alejandro Abdelnur wrote:

        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 20:34:17, Mona Chitnis wrote: > 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. Alejandro Abdelnur wrote: 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-08 23:06:09, Mona Chitnis wrote:

        > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38

        > <https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38>

        >

        > Aren't these 2 instances?

        Mona Chitnis wrote:

        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-08 15:08:40)

        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/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

        -------

        * New test case added to TestCoordSubmitXCommand.java for verifying the changes

        * Added additional test cases to TestELEvaluator.java as well

        Thanks,

        Srikanth

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-08 23:06:09, Mona Chitnis wrote: > trunk/core/src/test/resources/coord-multiple-output-instance4.xml, line 38 > < https://reviews.apache.org/r/4222/diff/2/?file=89137#file89137line38 > > > Aren't these 2 instances? Mona Chitnis wrote: 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-08 15:08:40) 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/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 ------- * New test case added to TestCoordSubmitXCommand.java for verifying the changes * Added additional test cases to TestELEvaluator.java as well Thanks, Srikanth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4222/

        -----------------------------------------------------------

        (Updated 2012-03-09 11:06:47)

        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/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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4222/ ----------------------------------------------------------- (Updated 2012-03-09 11:06:47) 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/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
        Hide
        Alejandro Abdelnur added a comment -

        Thanks Srikanth. Committed to trunk

        Show
        Alejandro Abdelnur added a comment - Thanks Srikanth. Committed to trunk

          People

          • Assignee:
            Srikanth Sundarrajan
            Reporter:
            Shwetha G S
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development