Addition of Error Flags: retriable and fatal #1533
Conversation
src/helm/proxy/retry.py
Outdated
| @@ -48,7 +48,13 @@ def retry_if_request_failed(result: Union[RequestResult, TokenizationRequestResu | |||
| """Fails if `success` of `RequestResult` or `TokenizationRequestResult` is false.""" | |||
| if not result.success: | |||
| hlog(result.error) | |||
| return not result.success | |||
| retry_if_fail: bool = ( | |||
| not hasattr(result, "error_flags") | |||
There was a problem hiding this comment.
don't think this is needed?
There was a problem hiding this comment.
it is because TokenizationRequestResult has no error_flages.
At some point I added the error_flags to TokenizationRequestResult but since it was not used, I thought it was bad practice.
Additionally, I seem to remember seeing that this function is called with other results than just RequestResult and TokenizationRequestResult (I think it was DecodeRequestResult). In the end, it became a nightmare to add these error_flags everywhere, so I thought this was a clean solution
There was a problem hiding this comment.
I changed it to isinstance(result, RequestResult)
|
@JosselinSomervilleRoberts this is nearly ready; let's get this merged soon |
|
Should be good now. @yifanmai do you want to check just before I merge? |
|
Looks good! |
Added ErrorFlags to handle errors that are consistent but not fatal.
This is used for Palmyra's content filtering.
For now handles #1520.