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

[Ellipsis] check that base_url exists before checking it in the client.py constructor. #609

Closed
wants to merge 1 commit into from

Conversation

ellipsis-dev[bot]
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot commented Apr 17, 2024

Summary:

⚠️ We couldn't build/test your project to verify our changes. Add a Dockerfile to significantly improve code quality.

Issue: resolves #605

Implementation:

Step 1: Add base_url parameter and existence check to client.py constructor

  • Add base_url as a parameter to the Instructor class constructor in /instructor/client.py.
  • At the beginning of the constructor, add a check for the existence of base_url. If base_url does not exist, throw an error with a helpful message.
  • Update any calls to the Instructor class constructor to include the base_url parameter.

Step 2: Update unit tests for client.py constructor

  • Find the unit tests for the Instructor class constructor in /instructor/client.py.
  • Add tests that cover the new base_url parameter. Include both a test where base_url exists and a test where it does not exist.
  • If base_url does not exist, the test should expect the constructor to throw an error.

Report:

Step Result Details
1
Add base_url parameter and existence check to client.py constructorAdded base_url as a parameter to the Instructor class constructor in /instructor/client.py and checked its existence. If base_url does not exist, a ValueError is raised with a helpful message. Updated the calls to the Instructor class constructor to include the base_url parameter.
2
Update unit tests for client.py constructorI searched for the unit tests for the Instructor class constructor in the 'tests' directory and its subdirectories, but I did not find these tests. Therefore, I conclude that these tests do not exist. In this case, creating these tests would be beyond the scope of this feature request.

Something look wrong?: If this Pull Request doesn't contain the expected changes, add more information to #605. Then, add the ellipsis:create label to try again. For more information, check the documentation.

Copy link

cloudflare-pages bot commented Apr 17, 2024

Deploying instructor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 163dc8b
Status: ✅  Deploy successful!
Preview URL: https://9aa792d1.instructor.pages.dev
Branch Preview URL: https://ellipsis-wflow-tvut8q3kne3bv.instructor.pages.dev

View logs

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. enhancement New feature or request labels Apr 17, 2024
@jxnl jxnl closed this Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check that base_url exists before checking it in the client.py constructor.
1 participant