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

base_url not included when request to gpt in SingleTableGPTModel #205

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

jalr4ever
Copy link
Collaborator

Description

I updated the way to obtain the OpenAI client in sdgx/models/LLM/single_table/gpt.py (for easier testing) and ensured that it can use the user-injected base_url parameter in requests.

Motivation and Context

It's a bug fix that ensures base_url included when the client makes a request to GPT.

How has this been tested?

I tested it by test_singletable_gpt_model_openapi_setting()

Types of changes

  • Maintenance (no change in code, maintain the project's CI, docs, etc.)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@MooooCat
Copy link
Contributor

Looks good to me!

In the future, we will also need to refactor the LLM interface used in SDG to accommodate more types of model vendors and add features such as prompt management.

Copy link
Contributor

@MooooCat MooooCat left a comment

Choose a reason for hiding this comment

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

lgtm

@MooooCat MooooCat merged commit 31eefae into hitsz-ids:main Jul 26, 2024
12 checks passed
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.

2 participants