Skip to content

Merge operation recorders#199

Open
shirzady1934 wants to merge 7 commits intodbos-inc:mainfrom
shirzady1934:merge-operation-recorders
Open

Merge operation recorders#199
shirzady1934 wants to merge 7 commits intodbos-inc:mainfrom
shirzady1934:merge-operation-recorders

Conversation

@shirzady1934
Copy link
Contributor

This PR addresses issue #148 by refactoring the system database to merge the functionality of recordChildGetResult into
recordOperationResult.

The recordOperationResult function has been updated to handle the parent-child relationship by accepting a childWorkflowID. The recordChildGetResult function has been removed, and its call sites have been updated to use the new recordOperationResult.

During the development process, a few other issue were identified and resolved a bug in the dynamic SQL query construction in the refactored recordOperationResult function was fixed.

Additionally, this PR addresses a race condition in the CLI integration tests that was causing flaky test runs. The tests were failing because they were checking for queued workflows before the parent workflow had finished enqueueing them. A polling mechanism has been added to wait until the expected number of workflows are present in the queue before proceeding with the assertions.

Copy link
Collaborator

@maxdml maxdml left a comment

Choose a reason for hiding this comment

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

Thanks @shirzady1934 for the PR.

@maxdml maxdml force-pushed the merge-operation-recorders branch from 47bc52a to 243fa52 Compare January 14, 2026 18:21
@maxdml maxdml force-pushed the merge-operation-recorders branch from 243fa52 to 0ba9bce Compare January 21, 2026 01:54
@maxdml
Copy link
Collaborator

maxdml commented Jan 21, 2026

@shirzady1934 one of the test does not pass: https://github.com/dbos-inc/dbos-transact-golang/actions/runs/21194325710/job/60966985052?pr=199

It looks like either we are not incrementing the step ID correctly, either there is a bug in how fork copies over the steps. This error might have been ignored in the past because record child output was incorrectly ignoring conflicts.

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.

2 participants