caikit / caikit-nlp

Apache License 2.0
12 stars 47 forks source link

Embedding retry returns the wrong exception #315

Closed markstur closed 9 months ago

markstur commented 9 months ago

Describe the bug

The retry (which was added to mitigate thread contention "already borrowed" errors) had some undesirable behavior. It was hiding actual errors. Also, now that we got that original problem fixed, this generally should be turned off so we won't eat-and-repeat on real errors.

Expected behavior

  1. If enabled and no success after the configured number of retries, then return the original exception instead of the one that says "too many retries". Use logging for the retry attempts.

  2. Default to zero retries because this is ideally no longer used (but can be enabled by env var if something comes up again).

Observed behavior

When actual errors that repeatedly failed happened, the exception was RuntimeError("Too many retries...") instead of a more usable error such as an ValueError.