Revert "refactor loopblock value" #1380
Conversation
Reverts Skyvern-AI/skyvern-cloud#3153 <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Reverts loop block value refactoring by removing specific exceptions and simplifying `ForLoopBlock` parameter handling. > > - **Exceptions**: > - Removed `FailedToFormatJinjaStyleParameter` and `NoIterableValueFound` exceptions from `exceptions.py`. > - **ForLoopBlock**: > - Reverted `loop_over` to be non-optional in `block.py`. > - Removed `loop_variable_reference` handling in `block.py` and `yaml.py`. > - Simplified `get_loop_over_parameter_values()` in `block.py` by removing Jinja template parsing. > - **Service**: > - Removed handling of `loop_variable_reference` in `service.py`. > - Adjusted `block_yaml_to_block()` to not consider `loop_variable_reference` in `service.py`. > > <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 75944d8fd7e03353612cddfed1396e8cb1e4ca93. 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.
❌ Changes requested. Reviewed everything up to 6000feb in 54 seconds
More details
- Looked at
199lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_8fikgSpkLdVof5Z2
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.
| status=BlockStatus.failed, | ||
| ) | ||
|
|
||
| loop_over_values = self.get_loop_over_parameter_values(workflow_run_context) |
There was a problem hiding this comment.
Removing exception handling for get_loop_over_parameter_values can lead to unhandled exceptions. Consider re-adding exception handling to provide clear error messages and prevent unexpected failures.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 6000feb in 1 minute and 18 seconds
More details
- Looked at
199lines of code in4files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/models/block.py:725
- Draft comment:
Removing exception handling in theexecutemethod ofForLoopBlockcan lead to unhandled exceptions. Consider adding back exception handling to provide clear error messages and prevent unexpected failures. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_pFJUiWTmzMqSXovx
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.
| @@ -748,15 +725,7 @@ async def execute_loop_helper( | |||
|
|
|||
| async def execute(self, workflow_run_id: str, **kwargs: dict) -> BlockResult: | |||
There was a problem hiding this comment.
Removing exception handling for get_loop_over_parameter_values can lead to unhandled exceptions during execution. Consider adding back exception handling to provide clear error messages and prevent unexpected failures.
| else: | ||
| raise NoIterableValueFound() | ||
| raise NotImplementedError |
There was a problem hiding this comment.
Raising NotImplementedError instead of a specific exception like NoIterableValueFound can make debugging more difficult. Consider using a more descriptive exception to improve error handling and clarity.
wintonzheng
deleted the
revert-3153-repo-sync/lawy/refactor-loopblock-value
branch
Reverts Skyvern-AI/skyvern-cloud#3153
Important
Reverts loop block value refactoring by removing specific exceptions and simplifying
ForLoopBlockparameter handling.FailedToFormatJinjaStyleParameterandNoIterableValueFoundexceptions fromexceptions.py.loop_overto be non-optional inblock.py.loop_variable_referencehandling inblock.pyandyaml.py.get_loop_over_parameter_values()inblock.pyby removing Jinja template parsing.loop_variable_referenceinservice.py.block_yaml_to_block()to not considerloop_variable_referenceinservice.py.This description was created by
for 6000feb. It will automatically update as commits are pushed.