enforce collection id for login parameters #1043
Conversation
![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! Reviewed everything up to 1a1ba58 in 15 seconds
More details
- Looked at
42lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/service.py:961
- Draft comment:
Ensurebitwarden_collection_idis checked for all relevant parameter types, not justBITWARDEN_LOGIN_CREDENTIAL. This includesBITWARDEN_SENSITIVE_INFORMATIONandBITWARDEN_CREDIT_CARD_DATAincreate_workflow_from_request. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_e6wM7d2gi9ZdAzmY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on fc5e561 in 25 seconds
More details
- Looked at
26lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_YmI5q2l73uo9ZSNy
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.
| @@ -707,6 +711,8 @@ async def create_workflow( | |||
| return await app.WORKFLOW_SERVICE.create_workflow_from_request( | |||
| organization=current_org, request=workflow_create_request | |||
| ) | |||
| except WorkflowParameterMissingRequiredValue as e: | |||
There was a problem hiding this comment.
Catching WorkflowParameterMissingRequiredValue and re-raising it is redundant. Consider removing this except block.
| except WorkflowParameterMissingRequiredValue as e: |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on e9b9cdb in 27 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_4xbKHqNy7qTEbo4G
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.
| @@ -753,6 +759,8 @@ async def update_workflow( | |||
| request=workflow_create_request, | |||
| workflow_permanent_id=workflow_permanent_id, | |||
| ) | |||
| except WorkflowParameterMissingRequiredValue as e: | |||
There was a problem hiding this comment.
Redundant exception handling for WorkflowParameterMissingRequiredValue. The exception is caught and immediately re-raised without any additional processing. Consider removing this except block.
| except WorkflowParameterMissingRequiredValue as e: |
Important
Enforces
bitwarden_collection_idrequirement forBITWARDEN_LOGIN_CREDENTIALparameters, raising an exception if missing.bitwarden_collection_idrequirement forBITWARDEN_LOGIN_CREDENTIALparameters increate_workflow_from_request()inservice.py.WorkflowParameterMissingRequiredValueifbitwarden_collection_idis missing.WorkflowParameterMissingRequiredValuetoexceptions.pyfor missing required workflow parameter values.WorkflowParameterMissingRequiredValueincreate_workflow()andupdate_workflow()inagent_protocol.py.This description was created by
for e9b9cdb. It will automatically update as commits are pushed.