DBOS "client context" #53
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… we'll make the encoder part of the context eventually.
maxdml
commented
Aug 12, 2025
Comment on lines
+302
to
+312
| // Our DB works with NULL values | ||
| var applicationVersion *string | ||
| if len(input.status.ApplicationVersion) > 0 { | ||
| applicationVersion = &input.status.ApplicationVersion | ||
| } | ||
|
|
||
| var deduplicationID *string | ||
| if len(input.status.DeduplicationID) > 0 { | ||
| deduplicationID = &input.status.DeduplicationID | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
I don't want to play with pointers in the codebase where we can distinguish variables through their zero values. Some rare cases, like the offset of listing workflows, must use pointers because 0 (the zero value of an int) is a valid offset.
maxdml
commented
Aug 12, 2025
maxdml
commented
Aug 12, 2025
maxdml
commented
Aug 12, 2025
| // This type check is redundant with the one in the wrapper, but I'd better be safe than sorry | ||
| typedInput, ok := input.(P) | ||
| if !ok { | ||
| // FIXME: we need to record the error in the database here |
Collaborator
Author
There was a problem hiding this comment.
Will fix this in another PR: if the wrapped workflow is invoked from the registry, and given a wrong input type, it'd error silently. This case can happen if the client enqueues a workflow with the wrong input type.
maxdml
commented
Aug 12, 2025
Comment on lines
+1248
to
+1250
| // Register the output for gob encoding | ||
| var r R | ||
| gob.Register(r) |
Collaborator
Author
There was a problem hiding this comment.
We need to perform gob encoding here, because the client might not be running in the same program where the workflow was registered -- and automatic registration was performed -- and I eventually want to move the encoder into the DBOSContext.
kraftp
reviewed
Aug 13, 2025
kraftp
approved these changes
Aug 13, 2025
… This will fallback to the latest step
Merged
maxdml
added a commit
that referenced
this pull request
Aug 14, 2025
Major fix: deadline computation for enqueued workflows. Previously we'd actually set the deadline when the workflow was enqueued, not dequeued. This PR fixes it by considering the timeout when setting the final deadline. (Note: we also revert the change from #53 and let `resumeWorkflow` clear the deadline. It'll be recomputed from the timeout in `RunAsWorkflow`.) Also: - Add a test `TimeoutOnlySetOnDequeue`, to verify the above fix - Record a workflow error when the function is called with the wrong parameter types. This could happen if the client enqueues a workflow with the wrong input type. - Prevent the spawning of child workflows within steps and add a test (`ChildWorkflowCannotBeSpawnedFromStep`) - Add a GHA to perform vulnerability checks and static analysis - Set toolchain to 1.25 (and update test GHA accordingly). Note this is *not* the minimum version required by library consumers. - Run `go vet` in test GHA - Add "join discord" badge to README - Make hardcoded values package level constants - Fix a small bug where returning the admin server object would be missing the port - Handle errors when writing the response in the healthcheck admin endpoint - Set `ReadHeaderTimeout` on the admin server to prevent Slowloris attacks - Handle sub millisecond timeouts (round to next millisecond) - Handle negative timeouts (can happen with propagation delays): set timeout to 1ms - Use a sync.Map for the workflow registry - Check the executable is a regular file before opening it to compute its hash - Remove unused WorkflowFunctionNotFound -- I don't see a use case for it - Use `struct{}{}` instead of `bool` for notification channels (`struct{}{}` use zero space) - Use an input struct for `dequeueWorkflows` - Cleanup handles implementation with a base handler that implement once shared logic (`GetStatus`, `GetWorkflowID`). other handles compose with this struct. - Handle errors when failing to close notification listener connection - Handle error when closing the connection used to set `LISTEN` channels in Postgres - Handle errors (and print a log entry) when failing to run a scheduled workflow - Fix possible integer overflows when converting uint to int (for queue priorities and fork start step) - Complete DLQ test: resume the DLQ-ed workflow and check it can now run to completion. Check that the completed workflow can be ran many times past the retry limit. - Pass `workflowCustomNametoFQN` to new contexts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds workflow management features usable by a "client context", that is, a DBOSContext object that is initialized but not launched. The benefits of doing so is being able to interact with DBOS workflows without having the process dequeue tasks, recover workflows, or run an admin server.
Features added:
Because the registry is indexed by workflow FQN, we need to keep another map from workflow "custom" name to workflow FQN. This should have been tested in PR that added custom workflow names.