-
Notifications
You must be signed in to change notification settings - Fork 177
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
Refactor OpenAI exception handling from instruction parser #328
Conversation
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.
oh - what was the breaking change? Happy to take a look as well @neubig |
No worries @krrishdholakia , probably just a minor conflict: https://github.com/neulab/prompt2model/pull/328/conflicts |
ah yes, just a minor conflict, will update my PR |
Ok, sounds good. Thanks for the merge! |
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.
I have one suggestion!
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.
Thanks, this was much clearer!
Description
Currently the
OpenAIInstructionParser
handles OpenAI exceptions from its own class. With this change, the exception is handled as required directly from theChatGPTAgent
. Refactoring the exception handling is a step in the direction of converting theOpenAIInstructionParser
to a more generalInstructionParser
class that can be passed any Generative Agent (OpenAI, cohere, local hf models, etc).Key Changes
handle_openai_error
returns the handled error instead of the number of API callsOpenAIInstructionParser
checks for the response being an exception or an actual response and continues the same logic as previously specified. This pattern will be extendable to future models.prompt_parser
test modified to mock the exact function that would be throwing the error in use.