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

automatically parse file extension when downloading #1101

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Oct 31, 2024

Important

Enhance download_file() in files.py to auto-detect file extensions from HTTP headers if missing in URL.

  • Behavior:
    • download_file() in files.py now attempts to determine file extension from HTTP headers if not present in URL.
    • Logs a warning if no extension is retrieved and logs an exception if retrieval fails.
  • Functions:
    • Adds get_file_extension_from_headers() to extract file extension from Content-Disposition or Content-Type headers.
  • Imports:
    • Adds mimetypes, re, and CIMultiDictProxy imports in files.py.

This description was created by Ellipsis for 504b44f. 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.

👍 Looks good to me! Reviewed everything up to 504b44f in 33 seconds

More details
  • Looked at 62 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. skyvern/forge/sdk/api/files.py:69
  • Draft comment:
    Typo: 'retreived' should be 'retrieved'. This occurs in multiple places (lines 69 and 71).
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The code uses 'retreive' instead of 'retrieve' in multiple places, which is a typo.
2. skyvern/forge/sdk/api/files.py:70
  • Draft comment:
    Catching a general exception is not a best practice. Consider catching specific exceptions that might occur when retrieving the file extension from headers.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code uses a try-except block to catch a general exception, which is not a best practice.
3. skyvern/forge/sdk/api/files.py:71
  • Draft comment:
    Typo: 'retreive' should be 'retrieve'. This occurs in multiple places (lines 69 and 71).
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The code uses 'retreive' instead of 'retrieve' in multiple places, which is a typo.

Workflow ID: wflow_fw6LEnS10DJIJRkY


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@LawyZheng LawyZheng merged commit d649699 into main Oct 31, 2024
2 checks passed
@LawyZheng LawyZheng deleted the lawy/automatically-parse-file-extension branch October 31, 2024 17:24
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