Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIU-368: Enable Drop persistence #249

Merged
merged 6 commits into from May 13, 2024
Merged

Conversation

myxie
Copy link
Collaborator

@myxie myxie commented May 6, 2024

Problem
This PR partially addresses LIU-368, which aims to enable proper persistence of data drops.

Currently, the persist and expireAfterUse flag are coupled together. This is orthogonal to the original behaviour, in which the expiry of the drop data should have no bearing on the persistence of the data (which would ensure it is copied elsewhere).

This PR partially addresses this work by de-coupling the expireAfterUse behaviour from the persist behaviour.

Solution

  • Remove persist check in dlm.expireCompletedDrops
  • Updated the default values in dlg.drop so that if we only use the lifespan if we don't specify expireAfterUse in the Drop arguments. Otherwise, we would set expireAfterUse to False and then the lifespan would be set, which is the opposite of what we want.

The current default behaviour exists:

  • FileDrop: expireAfterUse=False
  • MemoryDrop: ExpireAfterUse=True
  • Other Drops: ExpireAfterUse=None
    • If "lifespan" != -1, then the drop will still expire (after the timeout)
    • Else, the drop will not expire after use.

Testing

  • I've confirmed using a simple Hello World graph that setting the expireAfterUse to True will lead to the file.out being removed from the workspace; setting it to False leaves it present.
  • I've updated the test cases to be more explicit about the default behaviours for both File and Memory drops.

Future work
The 'persist' option has not been completely fulfilled. More work needs to be done to:

  • Add a file-store if the 'persist' option is selected
  • Establish a warning for persist when there is no persistent file-store specified.
  • Pickling/serialization mechansim for a MemoryDrop that is marked as persistent
  • Ability to load serialized MemoryDrops

myxie added 4 commits May 6, 2024 11:50
Separate out 'expireAfterUse' from 'persist'
Add expireAfterUse to Doxygen docstrings to expose to user.
Update comments to clarify expireAfterUse and Persist initial values.
@myxie myxie marked this pull request as ready for review May 6, 2024 09:18
myxie added 2 commits May 6, 2024 17:28
Files: expireAfterUse=False
Memory: expireAfterUse=True

This is irrespective of 'persist'
@awicenec awicenec merged commit 6207121 into LIU-368 May 13, 2024
10 of 16 checks passed
@awicenec awicenec deleted the LIU-368-updateExpireAfterUse branch May 13, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants