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

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!


- shwethags


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3752/#review4996
-----------------------------------------------------------


On 2012-02-09 09:26:21, shwethags wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3752/
> -----------------------------------------------------------
> 
> (Updated 2012-02-09 09:26:21)
> 
> 
> 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
>  1240005 
> 
> Diff: https://reviews.apache.org/r/3752/diff
> 
> 
> Testing
> -------
> 
> UT - TestCoordActionMaterializeCommand
> Tested with coord:current instance range and EL extension
> 
> 
> Thanks,
> 
> shwethags
> 
>

Reply via email to