beanlab / quest_framework

A Python framework for fault-tolerant workflows
0 stars 0 forks source link

Use deep copies of dictionaries in `InMemoryBlobStorage` #25

Open ABCambridge opened 7 months ago

ABCambridge commented 7 months ago

InMemoryBlobStorage needs to use deep copies of the dictionaries (storage) it is given so that they don't get wiped out when a WorkflowManager goes out of context.
See the following conversation from discord:

Andrew

I learned today that there can be some slight issues with using InMemoryBlobStorage. It is really convenient for running tests since there is no setup needed, but I did want to point out a curious bug incase one of you guys ever run into it. When you use InMemoryBlobStorage with a WorkflowManager, the key of workflow_id is associated with the _workflow_data dictionary belonging to the WorkflowManager. This works great until the WorkflowManager goes out of scope. Since the dictionary is passed around as a pointer object, the storage variable will now point workflow_id at nothing, since the previous _workflow_data no longer exists. This causes an issue specifically when you start a workflow in an async with statement, exit the code block (causing the WorkflowManager to call aexit), and then enter a new async with block to restart the WorkflowManager and resume the workflow. When you pick back up, storage is empty.

I was able to easily solve this by simply using a LocalFileSystemBlobStorage instead. It's a super simple change, but took a lot of debugging to figure out that that was specifically what was going on, so I wanted to save you guess the trouble in the future.

Gordon Bean

Ah - we need the InMemoryBlobStorage to make deep copies of the blobs passed into it. Can you make a ticket for that? We should use copy.deepcopy()

ABCambridge commented 6 months ago

This is being added to the sigint handling branch as part of that PR

ABCambridge commented 6 months ago

All it took was changing a single line. However, I thought it was weird that it still passed the tests even after I used InMemoryBlobStorage but before the change. Was I looking at the right file?