Skip to content

Comments

add retry functionality to extract_knowledge_from_text#49

Closed
wirthual wants to merge 1 commit intomicrosoft:mainfrom
wirthual:wirthual/retry
Closed

add retry functionality to extract_knowledge_from_text#49
wirthual wants to merge 1 commit intomicrosoft:mainfrom
wirthual:wirthual/retry

Conversation

@wirthual
Copy link
Contributor

Should there be a delay in between the calls? With a potential backoff?

@gvanrossum-ms
Copy link
Collaborator

gvanrossum-ms commented Oct 23, 2025

I'm sorry, this currently adds retries to knowledge extraction. However, that uses typechat, which already implements its own retry logic -- so we don't need to add our own.

I was thinking of adding it to the guts of AsyncEmbeddingModel, in get_embeddings_nocache() (the non-test branch towards the end). But we should first investigate whether AsyncOpenAI.embeddings.create already does or supports retries. (It has a timeout argument, but nothing to control retries that I can see.)

Assuming we have to do our own retries, the next thing to try is if its return value gives you access to the raw HTTP headers. There should be a header Retry-After:.

Maybe you can copy some logic from TypeAgent -- search for Retry-After.

@gvanrossum
Copy link
Collaborator

If no retry-after header is found we should definitely use exponential backoff, e.g. first retry after 1 second, second after 2s, then 4s, 8s etc.

@wirthual
Copy link
Contributor Author

Understood. Looks like AsyncOpenAI and AsyncAzureOpenAI support specifying the retries at initialization.

@gvanrossum
Copy link
Collaborator

Oh in that case we don't need to do anything. The default value is 2 retries, which brings the total number of tries to 3(*), which is exactly what I'd like to use.


(*) The code is pretty convoluted but I found code that loops over range(max_retries + 1) so clearly it tries three times if max_retries is set to two.

@gvanrossum gvanrossum closed this Oct 23, 2025
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.

3 participants