fix restart workflow run #1481
Merged
fix restart workflow run #1481
Conversation
This file contains 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
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Adds deletion of workflow run blocks during workflow run reset by introducing `delete_workflow_run_blocks` in `client.py` and updating `reset_workflow_run` in `workflows.py`. > > - **Behavior**: > - In `reset_workflow_run` in `workflows.py`, added call to `delete_workflow_run_blocks` to remove workflow run blocks during reset. > - **Database**: > - Added `delete_workflow_run_blocks` function in `client.py` to delete `WorkflowRunBlockModel` entries by `workflow_run_id` and `organization_id`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 475d8eb41f51d7f9ebffae0290d7a15daa8a9e39. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bc3fe61 in 43 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/db/client.py:2135
- Draft comment:
Consider handling the case whereorganization_idisNoneindelete_workflow_run_blocksto avoid unintended deletions. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
- The function accepts organization_id as optional (str | None)
- The SQL where clause uses organization_id directly without checking if it's None
- Looking at usage patterns in the codebase, organization_id is used similarly in many other functions
- SQLAlchemy handles None values in where clauses appropriately - it will match NULL values
- There's no evidence this has caused any issues
I could be wrong about SQLAlchemy's handling of None values in where clauses. Maybe there's a subtle issue I'm missing.
SQLAlchemy is well-known for handling None values correctly in where clauses, converting them to SQL NULL. The codebase uses this pattern extensively without issues.
Delete this comment. SQLAlchemy handles None values appropriately in where clauses, and this pattern is used consistently throughout the codebase.
Workflow ID: wflow_trY3Y4I3yoIMT5X2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to bc3fe61 in 54 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_A6S50ZAQ0ocqGEkL
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| stmt = delete(WorkflowRunBlockModel).where( | ||
| and_( | ||
| WorkflowRunBlockModel.workflow_run_id == workflow_run_id, | ||
| WorkflowRunBlockModel.organization_id == organization_id, |
There was a problem hiding this comment.
Consider handling the case where organization_id is None to avoid unintended deletions. You can use a conditional filter to only apply the organization_id filter if it is not None.
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.
Important
Adds
delete_workflow_run_blocksinclient.pyand updatesreset_workflow_runinworkflows.pyto delete workflow run blocks during reset.reset_workflow_runinworkflows.py, added call todelete_workflow_run_blocksto remove workflow run blocks during reset.delete_workflow_run_blocksfunction inclient.pyto deleteWorkflowRunBlockModelentries byworkflow_run_idandorganization_id.This description was created by
for bc3fe61. It will automatically update as commits are pushed.