feat(settings): Rate-limit setting updated to be per-profile #2376
Conversation
🦋 Changeset detectedLatest commit: 0f8b5f1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dosubot
bot
added
the
size:L
|
This pull request includes changes to both rate-limit settings and translations across multiple locales. To improve clarity and reviewability, it would be beneficial to split this into two separate pull requests:
By separating these concerns, it will be easier to review and test each set of changes independently. Thank you! |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| apiConfigs: { | ||
| default: { | ||
| id: this.defaultConfigId, | ||
| rateLimitSeconds: 0, |
There was a problem hiding this comment.
You'd have to ask gemini 😁. Actually I wasn't 100% sure when I saw it and forgot to come back to it by the time I'd gotten other things happy. Will remove it and make sure that things are cleanly dealing w/not having a value for rateLimitSeconds in the default.
mrubens
left a comment
mrubens
left a comment
There was a problem hiding this comment.
Just one question, looks and works great otherwise!
ross
left a comment
ross
left a comment
There was a problem hiding this comment.
The other question I had here was more around cleanup. The existing global version has to stay around, at least for now, to have something to migrate from, but it's plumbed through a bunch of places. I may make an additional pass through seeing what parts of that can go.
| apiConfigs: { | ||
| default: { | ||
| id: this.defaultConfigId, | ||
| rateLimitSeconds: 0, |
There was a problem hiding this comment.
You'd have to ask gemini 😁. Actually I wasn't 100% sure when I saw it and forgot to come back to it by the time I'd gotten other things happy. Will remove it and make sure that things are cleanly dealing w/not having a value for rateLimitSeconds in the default.
2 participants
Context
Move rate-limit setting from being a global setting in the advanced section to a per-profile setting.
Implementation
Created a new component RateLimitSecondControl that lives below TemperatureControl and behaves the same as it.
A new migrations "system" for ProviderSettingsMananager is introduced and it's set up to move the global rate-limit value over to any profiles that do not have an existing value on update.
Translations were AI-generated so I obviously can't vouch for those.
Screenshots
|
How to Test
Set a rate-limit in the advanced section before updating to this code. Update and see that the value you set is copied over to all of your profiles. You can then go into each profile and change the values to something that makes sense for the specific provider. Once saved the value should be tied to the provider.
If no value is set before you update, e.g. 0s, that 0s value will be copied to all the profiles as well.
Get in Touch
rwmcfa1
This replaces #1785 which appears to have stalled and in the meantime a lot of things changed out from under it making it difficult to use it as a starting point.
/cc Replaces #1785
/cc Fixes #2287 which requested this functionality
Important
Rate-limit setting updated to be per-profile with new UI component and migration system in
ProviderSettingsManager.ts.ProviderSettingsManager.ts.RateLimitSecondControlcomponent added belowTemperatureControlinApiOptions.tsx.ProviderSettingsManager.tsto transfer global rate-limit to profiles without existing values.RateLimitSecondsControladded toApiOptions.tsx.AdvancedSettings.tsx.ProviderSettingsManager.test.tsto cover migration logic.RateLimitSecondsControlinApiOptions.test.tsx.AdvancedSettings.tsxandSettingsView.tsx.This description was created by
for 425e5df. It will automatically update as commits are pushed.