-
Notifications
You must be signed in to change notification settings - Fork 5.9k
[app-server] read rate limits API #5302
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
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
b72858c to
e19f662
Compare
20a6979 to
9665d21
Compare
9665d21 to
b4aec26
Compare
| } | ||
|
|
||
| async fn fetch_account_rate_limits(&self) -> Result<RateLimitSnapshot, JSONRPCErrorError> { | ||
| let Some(auth) = self.auth_manager.auth() else { |
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.
Can we have some helper for internal errors? We use them quite a lot and they are verbose.
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.
will follow up in a separate PR
https://linear.app/openai/issue/CLI-861/app-server-create-helpers-for-internal-errors
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.
few nits, not blocking and some might be outside of this change's context
| std::fs::write(config_toml, format!("chatgpt_base_url = \"{base_url}\"\n")) | ||
| } | ||
|
|
||
| fn write_chatgpt_auth( |
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.
this is a lot of boilerplate unrelated to the feature, can we bump it into a shared helper? Do other tests do the same?
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.
created an auth_fixtures.rs helper file as an immediate step, but also will tackle a broader cleanup later: https://linear.app/openai/issue/CLI-862/app-server-fix-boilerplate-in-integration-tests
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.
seems like integration tests in app-server are pretty boilerplatey in general, so will have a refactor-focused PR
e0700b4 to
93bf412
Compare
Adds a
GET account/rateLimits/readAPI to app-server. This calls the codex backend to fetch the user's current rate limits.This would be helpful in checking rate limits without having to send a message.
For calling the codex backend usage API, I generated the types and manually copied the relevant ones into
codex-backend-openapi-types. It'll be nice to extend our internal openapi generator to support Rust so we don't have to run these manual steps.External (non-OpenAI) Pull Request Requirements
Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed:
https://github.com/openai/codex/blob/main/docs/contributing.md
If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes.