Fix cyclic import #781
Merged
Fix cyclic import #781
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 --> | 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 80b10a02f060431a1de292544116fe9659b6e2d7 | |--------|--------| ### Summary: Refactored code to fix circular import by removing `workflow` parameter from `cleanup_for_workflow_run` and handling `persist_browser_session` in `send_workflow_response`. **Key points**: - Removed `workflow` parameter from `cleanup_for_workflow_run` in `skyvern/webeye/browser_manager.py`. - Moved `persist_browser_session` logic to `send_workflow_response` in `skyvern/forge/sdk/workflow/service.py`. - Removed import of `Workflow` from `skyvern/webeye/browser_manager.py`. - Deleted `store_browser_session` method from `skyvern/webeye/browser_factory.py`. ---- Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev) <!-- 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 c2e37cc in 15 seconds
More details
- Looked at
85lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. skyvern/webeye/browser_manager.py:185
- Draft comment:
Remove the commented-outworkflowparameter from the function signature for clarity. - Reason this comment was not posted:
Confidence changes required:50%
The PR description mentions removing theworkflowparameter fromcleanup_for_workflow_runinbrowser_manager.py, but the function signature still has a comment indicating its removal. This comment should be removed for clarity.
2. skyvern/webeye/browser_factory.py:25
- Draft comment:
Remove the unused import ofappsincestore_browser_sessionmethod has been removed. - Reason this comment was not posted:
Confidence changes required:50%
The PR description mentions the removal of thestore_browser_sessionmethod frombrowser_factory.py. The method has been removed, but the import statement forappis still present and unused. This should be removed to clean up the code.
Workflow ID: wflow_sot8PJjt9aG7kw1j
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 c2e37cc in 15 seconds
More details
- Looked at
86lines of code in3files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_4voZICT3n8VSftTp
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.
| @@ -9,7 +9,7 @@ | |||
| from skyvern.constants import BROWSER_CLOSE_TIMEOUT | |||
| from skyvern.exceptions import MissingBrowserState | |||
| from skyvern.forge.sdk.schemas.tasks import ProxyLocation, Task | |||
| from skyvern.forge.sdk.workflow.models.workflow import Workflow, WorkflowRun | |||
| from skyvern.forge.sdk.workflow.models.workflow import WorkflowRun | |||
There was a problem hiding this comment.
The import of Workflow is now unused and should be removed to clean up the code.
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.
Summary:
Refactored code to fix circular import by adjusting
cleanup_for_workflow_runandsend_workflow_responsefunctions.Key points:
workflowparameter fromcleanup_for_workflow_runinskyvern/webeye/browser_manager.py.persist_browser_sessionlogic tosend_workflow_responseinskyvern/forge/sdk/workflow/service.py.Workflowfromskyvern/webeye/browser_manager.py.store_browser_sessionmethod fromskyvern/webeye/browser_factory.py.Generated with ❤️ by ellipsis.dev