-
Notifications
You must be signed in to change notification settings - Fork 227
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
retry openaicompatible requests if invalid content received #761
Conversation
One thought for try:
# ... existing model generation here ...
except JSONDecodeError as e:
logger.exception(e)
if self.retry_json:
raise GarakOpenAIBackoff from e
else:
raise e |
…on; refactor model calling and exception handling
garak/generators/openai.py
Outdated
if self.generator not in ( | ||
self.client.chat.completions, | ||
self.client.completions, | ||
): | ||
raise ValueError( | ||
"Unsupported model at generation time in generators/openai.py - please add a clause!" | ||
) |
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.
It looks like this was just moved so no action to take at this time. Just an inquiry to think on how to approach.
How can this occur? in theory _load_client
would raise an error not part of the backoff
set if it fails to set self.generator
.
Should we look for a way to enforce validation of this earlier in the run?
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.
It should be caught before here - it's def not intended to be mutable after init (ignoring the load/clear client mechanic). I guess _load_client
is a good place to check, yeah.
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.
_load_client
is a blank method in OpenAICompatible
which seems a distracting place to put this check; the check is already there in the OpenAIAPI
class. Moved it to __init__
, after the first _load_client
.
came back to a run and found this:
This patch assumes JSON output failures are transient, and so catches them with backoff. I would ideally like this to be configurable on/off - it's easy to imagine cases where both having it disable or enabled could be unexpected/frustrating. Conditional decorators look like a pain in Python, though.