> On 2012-02-09 23:59:47, Mohammad Islam wrote:
> > At first, I understand that you are mapping your new EL function as
> > CURRENT. Wandering about the code executed from line 201. Will that work
> > for you?
> >
> > Alternatively, what about this proposal.
> > getFuncType currently returns the function type based on fixed function
> > name --> function type mapping.
> > We can make it configurable through oozie-default/site.xml. By default, it
> > will be current-->CURRENT, latest->latest. But it could extended by
> > overriding the oozie properties in oozie-site.xml.
> >
> > It could be flexible for future custom EL functions too.
> >
> > The changes will mainly happen in oozie-default.xml, and getFunctionType
> > method.
> >
> > what is you take on this approach?
> >
> >
>
> shwethags wrote:
> Yes, materializeInstance(line 201) works for me. The main reason I mapped
> to CURRENT is for getInstanceNumber(). Since coord-action-create-inst context
> evaluation returns expression in terms of CURRENT, getInstanceNumber() parses
> the CURRENT expression and returns the correct instance number. So,
> materializeInstance() just passes that instance number and works fine.
>
> About your proposal, I am ok as long as resolveInstanceRange() works for
> me. Its not just getFuncType() which is a problem. You will have to re-look
> at getInstanceNumber also, or rather resolveInstanceRange on the whole for EL
> functions.
>
> shwethags wrote:
> Probably its a good idea to fix getFuncType like the way you suggested.
> getFuncType is used widely and fixing at one place will not help. We can
> probably use EL extension constants. Will you pick up this change or should
> I? Please let me know
>
> One suggestion: Function name should be parsed from the input(substring
> till '(') instead of just looking for substring with indexof. I have another
> EL function currentMonth which maps to beginning of month. Since it contains
> the string current, getFuncType thinks its coord:current!
Is the fix in getFuncType being incorporated? How is this mapping error getting
resolved otherwise?
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3752/#review4996
-----------------------------------------------------------
On 2012-03-07 09:43:36, shwethags wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3752/
> -----------------------------------------------------------
>
> (Updated 2012-03-07 09:43:36)
>
>
> Review request for oozie.
>
>
> Summary
> -------
>
> I have an EL extension today(0,0) which maps to start day of nominal time.
> This is used to specify startInstance, endInstance and instance in dataIn and
> dataOut of coordinator.
>
> In CoordCommandUtils.resolveInstanceRange(), getInstanceNumber has to return
> the instance number with respect to current. So, for coord-action-create-inst
> context, I have mapped today to current and hence getInstanceNumber returns
> the correct number. But later in resolveInstanceRange(), getFuncType is
> called with startInstance value which is today in this case and it maps to
> UNEXPECTED and throws up. getFuncType should be passed the evaluation of
> coord-action-create-inst context
>
>
> This addresses bug OOZIE-674.
> https://issues.apache.org/jira/browse/OOZIE-674
>
>
> Diffs
> -----
>
>
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
> 1297889
> trunk/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
> 1297889
>
> trunk/core/src/test/java/org/apache/oozie/command/coord/CoordELExtension.java
> PRE-CREATION
>
> trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordELExtensions.java
> PRE-CREATION
> trunk/core/src/test/resources/oozie-site-coordel.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3752/diff
>
>
> Testing
> -------
>
> UT - TestCoordActionMaterializeCommand
> Tested with coord:current instance range and EL extension
>
>
> Thanks,
>
> shwethags
>
>
