-
Notifications
You must be signed in to change notification settings - Fork 811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
try to fix upload file bug #695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 4ab3fa5 in 59 seconds
More details
- Looked at
30
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/webeye/actions/handler.py:759
- Draft comment:
The change in the order of operations within thefc_func
function might introduce issues if theis_filechooser_trigger
flag's state is important beforeset_files
is called. Consider reverting the order or ensuring that the flag's state is not used before this point. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_MTQwkg6z3qWGRU0q
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.
@@ -833,11 +833,11 @@ async def fc_func(fc: FileChooser) -> None: | |||
finally: | |||
LOG.info("Remove file chooser listener", action=action) | |||
|
|||
# Sleep for 10 seconds after uploading a file to let the page process it | |||
# Sleep for 15 seconds after uploading a file to let the page process it | |||
# Removing this breaks file uploads using the filechooser | |||
# KEREM DO NOT REMOVE | |||
if file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increasing the sleep duration from 10 to 15 seconds is a hardcoded solution to a timing issue. Consider implementing a more dynamic check to ensure the file has been processed, such as observing page changes or specific events.
4ab3fa5
to
cb98200
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on cb98200 in 58 seconds
More details
- Looked at
29
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_PUHtNneREnR86FrF
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.
nonlocal is_filechooser_trigger | ||
is_filechooser_trigger = True | ||
await fc.set_files(files=file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider ensuring the file setting operation is successful before setting is_filechooser_trigger
to True
. This can help avoid potential race conditions where the trigger is marked true but the file setting fails.
Summary:
Fixes file upload bug by adjusting file setting order and increasing post-upload sleep duration in
chain_click
.Key points:
chain_click
function ofskyvern/webeye/actions/handler.py
.fc_func
to set files afteris_filechooser_trigger
isTrue
.Generated with ❤️ by ellipsis.dev