Re: [Perl-workflow-devel] [rt.cpan.org #38024] all conditions with 'test' attirbute appears to be cached as 'evaluate'

[Jonas B. N. <jo...@gm...>]

Hi Robert,

Let me know what you username is on SourceForge. I cannot remember the  
process, perhaps you need to apply or something.

The project page is here:

	https://sourceforge.net/projects/perl-workflow/

I have created a branch named: test_condition

I can be reached via AIM: bjonasn or Jabber/Gtalk: jo...@gm...,  
if you require me to do something.

jonasbn

On 18/11/2008, at 21.10, Stockdale, Robert wrote:

> Jonas,
>
> I already have a SourceForge account so that is probably the easiest
> solution for everyone.
>
> Could you please grant me commit rights? Also, which branch would  
> you prefer
> I work in?
>
> --
> Bob Stockdale
> Administrative Computing Services
> University at Buffalo
> (716) 645-5334
> rf...@bu...
>
>
>> From: Jonas Brømsø Nielsen <jo...@gm...>
>> Date: Tue, 18 Nov 2008 14:45:59 -0500
>> To: Robert Stockdale <rf...@bu...>
>> Cc: "Brandt, Carl" <cb...@bu...>, Workflow development list
>> <per...@li...>
>> Subject: Re: [Perl-workflow-devel] [rt.cpan.org #38024] all  
>> conditions with
>> 'test' attirbute appears to be cached as 'evaluate'
>>
>> Hi Robert,
>>
>> You are more than welcome to get commit rights to our hosted
>> subversion repository.
>>
>> We are currently hosted at SourceForge so it requires an account  
>> there.
>>
>> Or you can send me the whole thing and I will sort it out.
>>
>> Thanks for taking the time and effort to do the extra iteration, I
>> think it will prove to be worth while.
>>
>> jonasbn
>>
>> On 18/11/2008, at 20.37, Stockdale, Robert wrote:
>>
>>> I attached the updated patch file to the RT ticket:
>>>
>>>   http://rt.cpan.org/Ticket/Display.html?id=38024
>>>
>>> Also, I wrote some automated tests for this change. What is the best
>>> way to
>>> get them into the distribution?
>>>
>>> --
>>> Bob Stockdale
>>> Administrative Computing Services
>>> University at Buffalo
>>> (716) 645-5334
>>> rf...@bu...
>>>
>>>
>>>> From: Jonas Brømsø Nielsen <jo...@gm...>
>>>> Date: Mon, 17 Nov 2008 15:22:42 -0500
>>>> To: "Brandt, Carl" <cb...@bu...>
>>>> Cc: Workflow development list <per...@li...>
>>>> Subject: Re: [Perl-workflow-devel] [rt.cpan.org #38024] all
>>>> conditions with
>>>> 'test' attirbute appears to be cached as 'evaluate'
>>>>
>>>> Sounds good.
>>>>
>>>> I was started to thinking along the lines of using Data::UUID to
>>>> generate the unique names, but that would probably just warp the
>>>> developers minds completely.
>>>>
>>>> I am looking for to the patch - we can get a development release  
>>>> out
>>>> of the door the moment it can be applied. This will also let other
>>>> developers evaluate the patch easily.
>>>>
>>>>
>>>> jonasbn
>>>>
>>>> On 17/11/2008, at 21.18, Jim Brandt wrote:
>>>>
>>>>> We discussed and came up with the following. Bob will be following
>>>>> up with a patch after some testing.
>>>>>
>>>>> * With regard to conditions, the name 'evaluate' is only used when
>>>>> creating the Condition::Evaluate object. It seems when conditions
>>>>> are evaluated, the sub just cycles through all conditions in the
>>>>> conditions array, so the name 'evaluate' isn't special in any way.
>>>>>
>>>>> * Naming for the auto-generated conditions
>>>>>
>>>>> When the condition fails, the name of the condition is written out
>>>>> to log4perl, so previous to the patch users would see that the
>>>>> 'evaluate' condition failed, which tells them that it was a 'test'
>>>>> condition, but does not tell them which one.
>>>>>
>>>>> Bob suggested even more detail in the name, like:
>>>>>
>>>>> "_$state\_$action\_condition\_$counter" so for example:
>>>>>
>>>>> _Begin_SA_condition_1
>>>>>
>>>>> If you had a problem, this would point you directly to the state-
>>>>>> action->in-line condition where your perl code is. I believe Bob
>>>>> said all of that information is easy to get inside the state  
>>>>> object.
>>>>>
>>>>>
>>>>> Jim
>>>>>
>>>>> Jonas Brømsø Nielsen wrote:
>>>>>> Hi Jim,
>>>>>> Thanks for the explanation, it was something along the lines of
>>>>>> what I
>>>>>> expected. Not being a user of this functionality however, I just
>>>>>> needed some hand-holding on the patch evaluation.
>>>>>> I will await your feedback from talking to Robert.
>>>>>> jonasbn
>>>>>> On 12/11/2008, at 21.37, Jim Brandt wrote:
>>>>>>> I'm ok with the principle of the patch. The special 'test'
>>>>>>> conditions are conditions with raw Perl code rather than an
>>>>>>> explicit
>>>>>>> condition object. The _create_condition_objects sub then  
>>>>>>> creates a
>>>>>>> condition object on the fly for each of these. The problem is  
>>>>>>> the
>>>>>>> name of each auto-generated condition object is hard-coded to
>>>>>>> 'evaluate'.
>>>>>>>
>>>>>>> I would want to verify if the 'evaluate' name is a magic name  
>>>>>>> used
>>>>>>> somewhere else in the code such that it expects it to be hard
>>>>>>> coded
>>>>>>> that way.
>>>>>>>
>>>>>>> Also, the patch uses the value of the test attribute as the  
>>>>>>> name.
>>>>>>> While this would guarantee that it will be unique, it could be  
>>>>>>> an
>>>>>>> arbitrarily large hash key (you can put in as much perl code as
>>>>>>> you
>>>>>>> want) and it wouldn't be very human readable if you were ever
>>>>>>> debugging.
>>>>>>>
>>>>>>> I would suggest adding some code to name it something like:
>>>>>>>
>>>>>>> "_" . <action_name> . "_condition_" . $counter
>>>>>>>
>>>>>>> where action_name is the name of the enclosing action and the
>>>>>>> counter is to allow us to have multiple 'test' type conditions  
>>>>>>> for
>>>>>>> an action. The leading underscore is just to indicate that is is
>>>>>>> private (auto-generated). So if you were debugging, the names of
>>>>>>> these condition objects would look something like:
>>>>>>>
>>>>>>> _MyAction_condition_1
>>>>>>>
>>>>>>> I work with Bob (Robert), so we'll discuss in the next day or  
>>>>>>> two.
>>>>>>>
>>>>>>> Jim
>>>>>>>
>>>>>>>
>>>>>>> Jonas Brømsø Nielsen wrote:
>>>>>>>> Hello All,
>>>>>>>> I have not received any feedback and I am reluctant to making a
>>>>>>>> development release for a one-line patch, I was hoping we would
>>>>>>>> be
>>>>>>>> able to evaluate this and decide simply on the contents of the
>>>>>>>> patch.
>>>>>>>> The issue is described below and the patch goes for line: 349  
>>>>>>>> in
>>>>>>>> HEAD
>>>>>>>> The line:
>>>>>>>>     name  => 'evaluate',
>>>>>>>> Roberts wants changed to:
>>>>>>>>     name => "_$condition_info->{test}",
>>>>>>>> I have not really gotten my head around the condition objects  
>>>>>>>> (I
>>>>>>>> should do this soon since our docs needs a serious update).
>>>>>>>> But this comment just above makes me hesitate applying the  
>>>>>>>> patch:
>>>>>>>>     # Special case: a 'test' denotes our 'evaluate' condition
>>>>>>>> So I guess the evaluate name is supposed to be hard-coded.
>>>>>>>> However
>>>>>>>> Roberts patch does seem to offer some flexibility. But then  
>>>>>>>> the _
>>>>>>>> (underscore) troubles me.
>>>>>>>> I am going to go over the tests covering this part later  
>>>>>>>> tonight,
>>>>>>>> but
>>>>>>>> if anybody has suggestions or opinions on this please feel free
>>>>>>>> to
>>>>>>>> write me (the list). Jim, I am particularly interested in your
>>>>>>>> opinion :)
>>>>>>>> Now I will go for my run to clear my head then I will look at  
>>>>>>>> the
>>>>>>>> code.
>>>>>>>> jonasbn
>>>>>>>> On 24/10/2008, at 23.55, Jonas Brømsø Nielsen wrote:
>>>>>>>>> Hiya Alll,
>>>>>>>>>
>>>>>>>>> We have received a patch from Robert Stockdale.
>>>>>>>>>
>>>>>>>>> If anybody has time to evaluate it, please feel free.
>>>>>>>>>
>>>>>>>>> jonasbn
>>>>>>>>>
>>>>>>>>> Begin forwarded message:
>>>>>>>>>
>>>>>>>>>> From: "Robert Stockdale via RT" <bug...@rt...>
>>>>>>>>>> Date: 29. sep 2008 21.48.42 GMT+02:00
>>>>>>>>>> To: undisclosed-recipients:;
>>>>>>>>>> Subject: [rt.cpan.org #38024] all conditions with 'test'
>>>>>>>>>> attirbute
>>>>>>>>>> appears to be cached as 'evaluate'
>>>>>>>>>> Reply-To: bug...@rt...
>>>>>>>>>>
>>>>>>>>>>  Queue: Workflow
>>>>>>>>>> Ticket <URL: http://rt.cpan.org/Ticket/Display.html? 
>>>>>>>>>> id=38024 >
>>>>>>>>>>
>>>>>>>>>> Hi Jonas,
>>>>>>>>>>
>>>>>>>>>> Sorry for the delay. I was able to confirm this bug.
>>>>>>>>>>
>>>>>>>>>> The name is being explicitly set to 'evaluate' in
>>>>>>>>>> Workflow::State::_create_condition_objects for all conditions
>>>>>>>>>> that
>>>>>>>>>> specify 'test' as an attribute in the xml configuration.  
>>>>>>>>>> This,
>>>>>>>>>> coupled
>>>>>>>>>> along with caching of condition results means that only the
>>>>>>>>>> first
>>>>>>>>>> 'evaluate' condition will actually be evaluated and each
>>>>>>>>>> subsequent
>>>>>>>>>> call
>>>>>>>>>> will use the cached result.
>>>>>>>>>>
>>>>>>>>>> Attached is a patch that will resolve the issue. Essentially
>>>>>>>>>> I am
>>>>>>>>>> just
>>>>>>>>>> taking the value of the 'test' attribute and naming the
>>>>>>>>>> condition
>>>>>>>>>> that,
>>>>>>>>>> with one small exception. Perl seems to have some issue with
>>>>>>>>>> keys
>>>>>>>>>> that
>>>>>>>>>> start with '!' so I prefixed it with an underscore. I tested
>>>>>>>>>> this
>>>>>>>>>> change
>>>>>>>>>> and it is working in my dev environment.
>>>>>>>>>>
>>>>>>>>>> An alternative option would be to allow the user to specify  
>>>>>>>>>> the
>>>>>>>>>> 'name'
>>>>>>>>>> attribute in the xml just like with any other condition. I  
>>>>>>>>>> did
>>>>>>>>>> not
>>>>>>>>>> exercise this option.
>>>>>>>>>>
>>>>>>>>>> -Bob
>>>>>>>>> <evaluate_condition_name.patch>
>>>>>>> --
>>>>>>> Jim Brandt
>>>>>>> Administrative Computing Services
>>>>>>> University at Buffalo
>>>>>>>
>>>>>
>>>>> --
>>>>> Jim Brandt
>>>>> Administrative Computing Services
>>>>> University at Buffalo
>>>>>
>>>>
>>>>
>>>> -------------------------------------------------------------------------
>>>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>>>> challenge
>>>> Build the coolest Linux based applications with Moblin SDK & win
>>>> great prizes
>>>> Grand prize is a trip for two to an Open Source event anywhere in
>>>> the world
>>>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>>>> _______________________________________________
>>>> Perl-workflow-devel mailing list
>>>> Per...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/perl-workflow-devel
>>>
>>
>