-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add custom URL option for Requesty provider #6984
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
Conversation
- Add requestyBaseUrl field to provider settings schema - Update RequestyHandler to use custom base URL when provided - Update fetcher functions to support custom base URL parameter - Add UI checkbox and text field for custom URL configuration - Update webview message handler to pass base URL to model fetcher - Add tests for custom base URL functionality Fixes #6983
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backward but somehow still broken.
|
|
||
| const url = "https://router.requesty.ai/v1/models" | ||
| const apiBaseUrl = baseUrl || "https://router.requesty.ai/v1" | ||
| const url = `${apiBaseUrl}/models` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider normalizing the URL to handle trailing slashes:
/models
This prevents potential double slashes if the base URL ends with a slash.
| setApiConfigurationField("requestyBaseUrl", "") | ||
| } | ||
| }}> | ||
| {t("settings:providers.useCustomBaseUrl")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The checkbox label says "Use custom base URL" but it could be more specific like "Use custom Requesty base URL" to match the field naming convention . This would provide better clarity when multiple providers are configured.
| {requestyBaseUrlSelected && ( | ||
| <VSCodeTextField | ||
| value={apiConfiguration?.requestyBaseUrl || ""} | ||
| type="url" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding URL validation before saving. While provides basic browser validation, you might want to ensure the URL is properly formatted (e.g., starts with http:// or https://) before allowing it to be saved.
| }) | ||
| }) | ||
|
|
||
| it("initializes with custom base URL when provided", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test coverage for the custom base URL! Consider adding edge case tests for:
- URL with trailing slash
- Invalid URL format
- Empty string handling when checkbox is unchecked
This PR adds support for custom base URLs in the Requesty provider, allowing users to connect to custom Requesty deployments (e.g., localhost or region-specific instances).
Changes
requestyBaseUrlfield to provider settings schemaRequestyHandlerto use custom base URL when provided (defaults tohttps://router.requesty.ai/v1)How it works
Testing
Fixes #6983
Important
Adds support for custom base URLs in the Requesty provider, allowing users to specify a different API endpoint.
requestyBaseUrlfield to provider settings schema inprovider-settings.ts.RequestyHandlerinrequesty.tsto use custom base URL if provided, defaulting tohttps://router.requesty.ai/v1.getRequestyModels()inrequesty.tsto accept a custom base URL.webviewMessageHandlerinwebviewMessageHandler.tsto pass base URL to model fetcher.Requesty.tsxfor custom URL configuration.requesty.spec.tsfor custom base URL initialization.GetModelsOptionstype inapi.tsto includebaseUrlfor Requesty.This description was created by
for f0aa301. You can customize this summary. It will automatically update as commits are pushed.