Skip to content

Check for the activity connection and use dispatchNow on sync queue#95

Closed
florisbosch wants to merge 2 commits into
durable-workflow:masterfrom
florisbosch:allow-sync-activities
Closed

Check for the activity connection and use dispatchNow on sync queue#95
florisbosch wants to merge 2 commits into
durable-workflow:masterfrom
florisbosch:allow-sync-activities

Conversation

@florisbosch
Copy link
Copy Markdown

@florisbosch florisbosch commented May 31, 2023

First of all very nice repository i've been working on a project which aims to do the same thing.
Although mine is not as complete as yours, yours is very well done!
If you're interested see: https://github.com/florisbosch/laravel-multi-stage-batch

This code change checks for the activity connection and utilizes dispatchNow on the sync queue if the connection is set to "sync". It then creates logs for the stored workflow and updates the context accordingly. In case the connection is not "sync," the code falls back to using dispatch. The context and index are incremented, and the Deferred class is used to return a promise.

This pull request which might be useful if you're working in your local development and have the queue connection set to "sync".

This pull requests solves the issue mentioned in #72

@rmcdaniel
Copy link
Copy Markdown
Member

Hey, thanks for figuring it out! Thanks for the PR too. I don’t think we should support the sync driver like this though because we won’t be able to make the timers work. Timers need to dispatch a job with a delay. For local development, I would at least recommend the database driver. I think sync should only be used during unit tests. Instead, maybe we should actually add an exception or a warning if you try to use the sync driver outside of unit tests.

@florisbosch
Copy link
Copy Markdown
Author

Hi,

Thanks for the info. Didn't know the timers had to be dispatched for the delays.

Some people might still always want to dispatch all activities in sync in that case you won't need the timers.
Maybe if we want to raise an error if you use sync it should be if not all activities are sync.

So its either all of them or none of them?

@rmcdaniel rmcdaniel closed this Jun 26, 2023
@Benjaminhu
Copy link
Copy Markdown

@rmcdaniel This warning / documentation mark would have saved me a few hours of debugging. I use "sync" mode by default in my development environment.

Instead, maybe we should actually add an exception or a warning if you try to use the sync driver outside of unit tests.

@rmcdaniel
Copy link
Copy Markdown
Member

It’s was added in the documentation under the requirements section that we don’t support the sync driver but I will prioritize adding the exception.

rmcdaniel pushed a commit that referenced this pull request May 8, 2026
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.

3 participants