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

Migrate from openai==0.28.1 to openai==1.2.4 #1407

Closed
wants to merge 1 commit into from

Conversation

johny-b
Copy link
Contributor

@johny-b johny-b commented Nov 14, 2023

I've tested a few evals and they seem to work.

Things I did:

  • run openai migrate
  • add "import openai" where it was necessary and removed by the migration script
  • change ["id"] to .id in registry.py
  • remove "api_base" and "api_key" arguments where they are no longer accepted
  • add model_dump() in both get_completion functions

NOTE: THIS IS NOT READY YET.

  • I don't know why we had "api_base" and "api_key" where I removed them - perhaps they should be now passed somewhere else?
  • some unit tests fail, seems related to the previous point
  • things could be prettier (e.g. we use client object in openai_completion_create_retrying but not in openai_chat_completion_create_retrying)
  • we get a stdout log for every API request which is not convenient
  • it seems that now when we exceed the context window, we're repeating the request (issue]
[2023-11-14 11:09:32,241] [_common.py:105] Backing off openai_completion_create_retrying(...) for 17.0s (openai.BadRequestError: Error code: 400 - {'error': {'message': "This model's maximum context length is 8001 tokens, however you requested 8184 tokens (7672 in your prompt; 512 for the completion). Please reduce your prompt; or completion length.", 'type': 'invalid_request_error', 'param': None, 'code': None}})

I've tested a few evals and they seem to work.

Things I did:

* run openai migrate
* add "import openai" where it was necessary and removed by migration
  script
* change ["id"] to .id in registry.py
* remove "api_base" and "api_key" arguments where they are no longer
  accepted
* add model_dump() in both get_completion functions

NOTE: THIS IS NOT READY YET. E.g. I don't know why we had "api_base" and
"api_key" where I removed - perhaps they should be now passed somewhere
else?
@etr2460
Copy link
Collaborator

etr2460 commented Dec 5, 2023

Closing as we've resolved this in #1420.

Thanks for starting this work here and encouraging us to finish this upgrade!

@etr2460 etr2460 closed this Dec 5, 2023
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.

2 participants