go-openai icon indicating copy to clipboard operation
go-openai copied to clipboard

feat: Admin Management Functionality

Open MattDavisRV opened this issue 8 months ago • 3 comments

A similar PR may already be submitted! Please search among the Pull request before creating one.

If your changes introduce breaking changes, please prefix the title of your pull request with "[BREAKING_CHANGES]". This allows for clear identification of such changes in the 'What's Changed' section on the release page, making it developer-friendly.

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request.

Describe the change This change includes functionality to manage your Org via Admin API. This change does not include all endpoints and functionality related to the Admin API. As I have time, and if no one else has added them, I will submit another PR for the missing functionality.

Provide OpenAI documentation link Admin API Documentation

Describe your solution Implemented functionality to:

  • Manage Admin API Keys
  • Manage Organization Invites
  • Manage Organization Users
  • Manage Projects
  • Manage Project Users
  • View Organization Costs

Tests Tests have been created for all functionality added. Test methodology was copied from existing tests in the library.

Additional context I'm unsure why the Sanity checks are failing, they were locally as well, but unrelated to my changes (jsonschema).

Issue: #XXXX

MattDavisRV avatar Mar 11 '25 17:03 MattDavisRV

Codecov Report

Attention: Patch coverage is 67.92453% with 119 lines in your changes missing coverage. Please review.

Project coverage is 83.01%. Comparing base (d7dca83) to head (3d2b63c). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
examples/admin_usage/main.go 0.00% 47 Missing :warning:
admin_invite.go 74.13% 10 Missing and 5 partials :warning:
admin_project.go 77.27% 10 Missing and 5 partials :warning:
admin_project_user.go 77.27% 10 Missing and 5 partials :warning:
admin_key.go 76.00% 8 Missing and 4 partials :warning:
admin_user.go 76.47% 8 Missing and 4 partials :warning:
admin_usage.go 90.90% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
- Coverage   85.43%   83.01%   -2.42%     
==========================================
  Files          43       50       +7     
  Lines        2313     2684     +371     
==========================================
+ Hits         1976     2228     +252     
- Misses        315      410      +95     
- Partials       22       46      +24     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 11 '25 18:03 codecov[bot]

I add additional test cases to get a better coverage of the implementation. I believe this is failing the coverage now due to the example included. I see that other examples do not have test written for them, and I didn't see an obvious way to exclude it from being checked.

MattDavisRV avatar May 07 '25 17:05 MattDavisRV

@sashabaranov any chance this can be looked at?

MattDavisRV avatar Jun 12 '25 23:06 MattDavisRV

@sashabaranov sorry for the ping again on this, would really like to get this merged so I can setup some automation on my side without using my fork. If you have any advice on how to exclude the examples from the code coverage, which seems to be dropping the overall coverage, happy to take a look at it.

MattDavisRV avatar Jul 01 '25 10:07 MattDavisRV

Hi @MattDavisRV!

Sorry for being so slow with this. This is a fantastic PR that I'm finding hard to chew. The 1821 lines added are non-trivial to review, especially with the fact that there is no good integration testing for this!

Can we break this down into smaller pieces and merge one by one?

No reduction in unit test coverage would also help!

sashabaranov avatar Jul 01 '25 14:07 sashabaranov

Sure, I'll break this up into smaller PRs, I'll start with the usage one as that's what I need for my use case.

MattDavisRV avatar Jul 01 '25 14:07 MattDavisRV