Skip to content
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

fix download file extension #1157

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Nov 7, 2024

Important

Fixes file renaming in execute_step() in agent.py to retain original file extensions for downloaded files.

  • Behavior:
    • Fixes file renaming in execute_step() in agent.py to append the original file extension to the new file name.
    • Ensures downloaded files retain their original extensions when renamed.

This description was created by Ellipsis for 844f8a9. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 844f8a9 in 1 minute and 34 seconds

More details
  • Looked at 26 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/agent.py:302
  • Draft comment:
    Ensure that the file extension is preserved when renaming files by appending file_extension to random_file_name. This change is correct and necessary to maintain the integrity of the downloaded files.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR fixes the issue of missing file extensions when renaming downloaded files. The change is correct and aligns with the intent to preserve file extensions.

Workflow ID: wflow_cszXcHNe2oGGMM9r


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.

@@ -293,11 +294,12 @@ async def execute_step(
if len(list_files_after) > len(list_files_before):
files_to_rename = list(set(list_files_after) - set(list_files_before))
for file in files_to_rename:
file_extension = Path(file).suffix
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code duplicates existing functionality for extracting file extensions using Path(filename).suffix in skyvern/forge/sdk/api/files.py. Consider reusing or extending the existing code to avoid duplication.

@LawyZheng LawyZheng merged commit 45477ab into main Nov 7, 2024
2 checks passed
@LawyZheng LawyZheng deleted the lawy/fix-download-file-extension branch November 7, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant