Skip to content

Conversation

@ehuelsmann
Copy link
Member

Description

Document that the serializer is expected to persist the context as well as the immediate workflow state. I'm explicitly declaring that it's not the Context instance which is to be returned, because that creates a tight coupling between the persister and the context class it will use. At a later stage, the Factory can be used to associate different Context classes with workflows.

Also, update the File persister to persist the workflow's Context according to the declared protocol.

Note: we probably needs something similar in the DBI serializer?

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Marking as 2.0, because this breaks older serializations: When a Context object was returned from the persister, the new context being instantiated by the Factory receives the content of the blessed Context hash instead of the PARAMS hash.

Checklist:

  • My code follows the style guidelines of this project, please see the contribution guidelines.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@ehuelsmann ehuelsmann added this to the 2.0 milestone Jan 15, 2022
@ehuelsmann ehuelsmann requested a review from jonasbn January 15, 2022 20:46
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 92.446% when pulling 41d2438 on ehuelsmann:persister-context-docs into 8ae8d0e on jonasbn:master.

Copy link
Collaborator

@jonasbn jonasbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@ehuelsmann ehuelsmann merged commit d091ea4 into perl-workflow:master Jan 24, 2022
@ehuelsmann ehuelsmann deleted the persister-context-docs branch January 24, 2022 21:09
@jonasbn
Copy link
Collaborator

jonasbn commented Jan 25, 2022

@ehuelsmann you have marked this for 2.0 milestone, could it not be a part of the 1.58 release?

@ehuelsmann
Copy link
Member Author

@jonasbn I don't think that's a good idea. I listed this as a breaking change because it changes how workflows are being serialized, which I'd expect to cause problems for people with stored contexts like the contexts stored through the 'file persister': the contract changed from returning an instantiated context to returning the keys that are to be used to instantiate a context.

In regular Perl objects, we'd be able to do %$object to get the keys we need, but with Workflow::Base that's not the case: it stores its values in PARAMS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants