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: remove limit param from openai FileObject.list() #505

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

aakashb09
Copy link
Contributor

@aakashb09 aakashb09 commented Mar 14, 2024

fixes #503


Ellipsis 🚀 This PR description was created by Ellipsis for commit 323ed97.

Summary:

This PR removes the limit parameter from the get_files function, affecting the list function in instructor/cli/files.py, to return and display all files.

Key points:

  • Removed limit parameter from get_files function in instructor/cli/files.py.
  • Updated list function to call get_files without limit parameter.

Generated with ❤️ by ellipsis.dev

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 the entire pull request up to 323ed97
  • Looked at 50 lines of code in 1 files
  • Took 49 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 85%.
1. instructor/cli/files.py:39:
  • Assessed confidence : 80%
  • Comment:
    Removing the limit parameter could potentially lead to performance issues if there are a lot of files. Consider adding a default limit and an optional limit parameter to allow the user to specify the number of files they want to fetch and list.
  • Reasoning:
    The PR removes the limit parameter from the get_files() function and the list() function. This means that all files will be fetched and listed, which could potentially be a large number. This could lead to performance issues if there are a lot of files. However, the PR author might have removed the limit parameter because they want to fetch and list all files, regardless of the number. I will check the issue Unexpected keyword argument 'limit' in files list CLI #503 that this PR is supposed to fix to understand the context better.

Workflow ID: wflow_NDnHq2GR4JJGjy28


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

@jxnl jxnl merged commit 4c24718 into instructor-ai:main Mar 14, 2024
@aakashb09 aakashb09 deleted the fix/get-files branch March 14, 2024 15:43
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.

Unexpected keyword argument 'limit' in files list CLI
2 participants