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

feat(basePath): enable base path for configuration #69

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

metrue
Copy link

@metrue metrue commented May 16, 2023

What

This MR is to resolve the issue #68 (comment)

Why

When we set up a proxy for OpenAI API, sometimes we enable a custom endpoint for it, so enabling a base path for the configuration would be helpful when we have something like 'my.host.com/openai/api' proxying OpenAI API.

Affected Areas

Configuration to initialize the OpenAI API instance but it's backward compatible.

Copy link
Collaborator

@Krivoblotsky Krivoblotsky left a comment

Choose a reason for hiding this comment

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

Let's improve paths logic 🙏

Sources/OpenAI/OpenAI.swift Outdated Show resolved Hide resolved
Sources/OpenAI/OpenAI.swift Outdated Show resolved Hide resolved
Sources/OpenAI/OpenAI.swift Outdated Show resolved Hide resolved
@metrue
Copy link
Author

metrue commented May 30, 2023

Let's improve paths logic 🙏

Done refactoring and unit tests added.

@metrue metrue requested a review from Krivoblotsky June 13, 2023 12:20
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@WalterZou
Copy link

The functionality of this PR is needed, but it seems like Merge is stuck @metrue @Krivoblotsky

@Krivoblotsky
Copy link
Collaborator

Hey, everyone!
Thanks a lot for the contribution, I'll review the next batch of PRs today (including this one) and publish a new release.

@Krivoblotsky
Copy link
Collaborator

Also, I need to check why the build is failing ⚠️

@WalterZou
Copy link

Hey, everyone! Thanks a lot for the contribution, I'll review the next batch of PRs today (including this one) and publish a new release.

Cool, thanks so much 🙏

@Krivoblotsky
Copy link
Collaborator

Hey, @metrue.
Could you please fix the build? 🙏

@metrue
Copy link
Author

metrue commented Aug 23, 2023

hey @Krivoblotsky

I checked the CI log, it does tell why it failed,

2023-06-20T16:21:08.9885513Z Build complete! (4.18s)
2023-06-20T16:21:09.5590135Z error: Exited with signal code 4
2023-06-20T16:21:09.5631060Z Test Suite 'All tests' started at 2023-06-20 16:21:09.008
2023-06-20T16:21:09.5631973Z Test Suite 'debug.xctest' started at 2023-06-20 16:21:09.317
2023-06-20T16:21:09.5632912Z Test Suite 'OpenAITests' started at 2023-06-20 16:21:09.317
2023-06-20T16:21:09.5633428Z Test Case 'OpenAITests.testAudioTranscriptions' started at 2023-06-20 16:21:09.317
2023-06-20T16:21:09.5698541Z ##[error]Process completed with exit code 1.
2023-06-20T16:21:09.5791394Z Post job cleanup.
2023-06-20T16:21:09.7007792Z [command]/usr/bin/git version

And I run swift test locally, it works well, anyone can advice would be great.

Test Case '-[OpenAITests.OpenAITestsDecoder testModerations]' started.
Test Case '-[OpenAITests.OpenAITestsDecoder testModerations]' passed (0.000 seconds).
Test Suite 'OpenAITestsDecoder' passed at 2023-08-23 22:47:56.335.
	 Executed 10 tests, with 0 failures (0 unexpected) in 0.003 (0.003) seconds
Test Suite 'OpenAIPackageTests.xctest' passed at 2023-08-23 22:47:56.335.
	 Executed 46 tests, with 0 failures (0 unexpected) in 0.029 (0.030) seconds
Test Suite 'All tests' passed at 2023-08-23 22:47:56.335.
	 Executed 46 tests, with 0 failures (0 unexpected) in 0.029 (0.033) seconds

@Krivoblotsky
Copy link
Collaborator

@metrue, thanks!
CI runs build and test on Linux, maybe this is the cause. I'll try to run it once again and check.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tisfeng
Copy link

tisfeng commented Jan 1, 2024

Hi, how is this PR coming along? Customizing the endpoints is necessary and I'm looking forward to this feature.

@tisfeng
Copy link

tisfeng commented Jan 1, 2024

Going a step further, we can customize the entire endpoint, not just the base path, because sometimes we build the OpenAI service locally, when it's an http request instead of an https, e.g. http://localhost:3000/v1/chat/completions

Therefore, I hope to provide an endPoint property that allows the user to freely configure the request URL.

image

@snake302
Copy link

snake302 commented Jan 5, 2024

I need this

@tisfeng
Copy link

tisfeng commented Feb 16, 2024

Hi, how's this going?

@qaz1991815
Copy link

还没搞定吗?

Copy link

Quality Gate Passed Quality Gate passed

Issues
8 New issues

Measures
0 Security Hotspots
No data about Coverage
1.3% Duplication on New Code

See analysis details on SonarCloud

@tisfeng
Copy link

tisfeng commented May 11, 2024

Due to the lack of developers handling this issue, I added a new public API myself to make it easier to support user-defined request URLs, just for my own convenience, and also submitted a PR #204 .

Can be used simply as follows:

openAI.chatsStream(query: query, url:"http://localhost:11434/v1/chat/completions")

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.

6 participants