Thread: [Perl-workflow-devel] Patch from Heiko (RT #64940)
Brought to you by:
jonasbn
From: Jonas B. N. <jo...@gm...> - 2011-01-29 16:39:57
|
Hello All, We have received a patch from Heiko Schlitterman Please see RT queue: https://rt.cpan.org/Ticket/Display.html?id=64940 The issue being fixed is longevity of workflow context. The documentation does not say anything and it seems the context is more a scratchpad. At the same time I think Heiko's take is pretty okay - but I do not want to break anything. Please let me know what you think, jonasbn -- AIM: BjonasN Gtalk: jo...@gm... ICQ: 62401545 MSN: jo...@io... twitter: jonasbn blog: http://e-diot.dk/ |
From: Alejandro I. <ai...@p2...> - 2011-01-29 17:05:52
|
On Sat, Jan 29, 2011 at 11:39 AM, Jonas B. Nielsen <jo...@gm...> wrote: > Hello All, > > We have received a patch from Heiko Schlitterman > > Please see RT queue: https://rt.cpan.org/Ticket/Display.html?id=64940 > > The issue being fixed is longevity of workflow context. > > The documentation does not say anything and it seems the context is more a scratchpad. At the same time I think Heiko's take is pretty okay - but I do not want to break anything. > > Please let me know what you think, > It seems good but I agree with you: Context is a meant as a disposable stash akin to TT's stash, catalysts's stash, etc. if you need to save stuff from the context, that is _precisely_ what the ExtraData extension is for, so no need to add this to the core IMHO. I do understand their need however, because I do a similar thing in p2ee. We save some of the stuff from the context in our workflows as "state_data" is a JSON string using the standard extension mechanisms already provided by Workflow.pm namely Workflow::Persister::DBI::ExtraData, here is our persister config: <persisters> <persister name="Inventory" class="Workflow::Persister::DBI::ExtraData" dsn="DBI:Pg:dbname=p2ee" user="inventory" password="inventory" date_format="%Y-%m-%d %H:%M" autocommit="1" workflow_table="wf" workflow_sequence="wf_workflow_id_seq" history_table="wf_history" history_sequence="wf_history_workflow_hist_id_seq" extra_table="wf_p2ee" extra_data_field="creator,citho,citho_since" /> </persisters> So my vote is -1 on this incorporating this patch because this is already avail in the core capabilities via ExtraData. It would be interesting nevertheless to have an extension and standard mechanism to give context persistence (but people are probably storing all sorts of stuff like references into the context so I can't envision how to make a general case). But before investing in extensions to the current code I would rather invest efforts to port Workflow to Moose first. I do vote +1 however to port to Moose, though OT to this thread ;-) Best, Alex |
From: Martin B. <vc...@cy...> - 2011-01-29 20:31:02
|
Hi Jonas, > We have received a patch from Heiko Schlitterman > > Please see RT queue: https://rt.cpan.org/Ticket/Display.html?id=64940 > > The issue being fixed is longevity of workflow context. > > The documentation does not say anything and it seems the context is more a scratchpad. At the same time I think Heiko's take is pretty okay - but I do not want to break anything. as you might remember I am working for the OpenXPKI project which is using Workflow as a central primitive which forms the glue for all the PKI related components. Back in 2006 or so I wrote an OpenXPKI specific persister for Workflow which is used instead of the original one. This custom persister does exactly what Heiko mentions: it makes sure that all Workflow context data is persisted to the database. Back then I noticed this was not the case and figured I was supposed to do it myself - what good is a workflow that loses information when persisted? So we are definitely relying on persistent workflow context entries. For scratch data we have introduced the notion of "volatile" context entries. In our case, if the context key name starts with an underscore the persister will not write the value to database. This allows us to safely transport sensitive data between workflow activities (knowing they will not get persisted). Our policy is that an activity must not expect a required volatile input context value to exist (it might just have been resurrected by the factory). Instead, it must be prepared to compute the value itself if it is not present. Basically, if you wish to change the behavior to what Heiko suggested, we agree that this is useful. And since we have our own persister, this change will not affect us at all. HTH, Martin |
From: Andrew O'B. <an...@or...> - 2011-01-31 00:20:59
|
Hi Alejandro, > > I do vote +1 however to port to Moose, though OT to this thread ;-) Why on earth would you slow down all applications that rely on Workflow, not to mention adding the dependencies, like this? If there is a refactoring in the works then perhaps look at whatever compatible module du jour (Mouse? Or was there another newer one?) that provides the most common features without 90% of the bloat. Just a thought. Cheers, Andrew |
From: Alejandro I. <ai...@p2...> - 2011-01-31 13:57:26
|
On Sun, Jan 30, 2011 at 6:52 PM, Andrew O'Brien <an...@or...> wrote: > Hi Alejandro, > >> >> I do vote +1 however to port to Moose, though OT to this thread ;-) > > Why on earth would you slow down all applications that rely on Workflow, not to mention adding the dependencies, like this? > Hi Andrew, I could agree on the dependencies but I don't think there would be a performance penalty at all, or only marginal at best, and it's mostly a compile-time issue. Workflow engines should be long running instances anyway with a per-request light-weight object factory, much like the current code, so I don't think this would really be an issue, IMHO. Workflow's code is meant to be extended and there are many things that would benefit from a more formal object protocol. For example the WF fields could greatly benefit from a type system (Int, Num, Str, etc.), roles, etc. It would also point the code towards Perl 6. > If there is a refactoring in the works No, there is no refactoring in the works AFAIK. It was just a thought, to make the WF code easier to extend and maintain. >then perhaps look at whatever compatible module du jour (Mouse? Or was there another newer one?) that provides the most >common features without 90% of the bloat. > Don't get biased by sparse comments here and there around Moose. It's true that Moose is a heavy-weight complete object system but people that are creating this FUD around Moose fail to mention namespace autocleaning, non-mutability and other best practicesto keep the final code running lean. There is a factual penalty on compile time, however, and even so, in complex code it's really not that noticeable. A perfect example is Catalyst which was ported to Moose a while back and I have not personally seen any performance penalty, and we do heck of a lot of development in Catalyst. Even for development, the compile time for re-starting a Catalyst app before or after Moose didn't really change that much, and definitively we haven't seen _any_ performance penalties in run-time. On the contrary, the Cat Moose code allows for more easy creation and maintenance of the Model layer, so you can more easily design the model classes as long-running "instances" that create light-weight per-request objects. And supposing there is in fact a marginal run-time performance penalty, I think the extensibility and maintainability would greatly out-weight it, again IMHO. In any case, it wouldn't affect current users of WF except for the fact that they could more easily extend and maintain their adaptations of, and help in the publications of extensions like Context persistence onto the CPAN more easily. And finally, the WF code in general is aging and perhaps a good time to re-evaluate some code design and architectural choices as well. Maybe an HTTP/Resource based approach with JSON data ala CouchDB ? Could be a branch/fork as well. -- Alejandro > Just a thought. > > Cheers, > > Andrew > > > |
From: Alejandro I. <ai...@p2...> - 2011-01-31 14:03:56
|
On Mon, Jan 31, 2011 at 8:57 AM, Alejandro Imass <ai...@p2...> wrote: > On Sun, Jan 30, 2011 at 6:52 PM, Andrew O'Brien <an...@or...> wrote: >> Hi Alejandro, >> [...] > > Hi Andrew, > > I could agree on the dependencies but I don't think there would be a [...] > persistence onto the CPAN more easily. And finally, the WF code in > general is aging and perhaps a good time to re-evaluate some code > design and architectural choices as well. Maybe an HTTP/Resource based > approach with JSON data ala CouchDB ? Could be a branch/fork as well. > Maybe Workflow::REST ??? This way current users have a choice whether to use Standard or REST-based, whoch would be a complete port/re-write. Things we do on the REST version could be ported to Standard with Mouse, or even wrap Standard WF with MooseX::NonMoose as an intermediate step, or something along those lines. -- Alex |
From: Kaare R. <ka...@ja...> - 2011-01-31 21:41:17
|
Hi Alex > This way current users have a choice whether to use Standard or > REST-based, whoch would be a complete port/re-write. Things we do on As I understand it, backwards compatibility is a very high priority for Workflow. If you want to include Moose and start rewriting tings, it may be better to start all over with a clean slate. That includes the most difficult task of all: Finding a new name!! ;-) I'd be interested in doing a bit of brainstorming, as I may be in need for a sleeker Workflow engine (or just a state machine), but I think it may be off topic here. -- Med venlig hilsen Kaare Rasmussen, Jasonic Jasonic Telefon: +45 3816 2582 Nordre Fasanvej 12 2000 Frederiksberg Email: ka...@ja... |
From: Alejandro I. <ai...@p2...> - 2011-01-31 22:09:09
|
On Mon, Jan 31, 2011 at 4:25 PM, Kaare Rasmussen <ka...@ja...> wrote: > Hi Alex > >> This way current users have a choice whether to use Standard or >> REST-based, whoch would be a complete port/re-write. Things we do on > > As I understand it, backwards compatibility is a very high priority for > Workflow. If you want to include Moose and start rewriting tings, it may be > better to start all over with a clean slate. That includes the most difficult > task of all: Finding a new name!! ;-) But I see no problem in using the same Workflow namespace! Jonas, Would you have a problem of a RESTful branch of Workflow in the Workflow namespace? Something like Workflow::REST ? My rationale is that instead of forking it would be better to work collaboratively with the existing Workflow project and community, but use separate namespace, we could even use the same subversion repo in a different branch. |