feat: Enhance error handling in Azure document embedder #8941
Conversation
github-actions
bot
added
topic:tests
type:documentation
Pull Request Test Coverage Report for Build 13550304131Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| try: | ||
| if self.dimensions is not None: | ||
| response = self._client.embeddings.create( | ||
| model=self.azure_deployment, dimensions=self.dimensions, input=batch | ||
| ) | ||
| else: | ||
| response = self._client.embeddings.create(model=self.azure_deployment, input=batch) | ||
|
|
||
| # Append embeddings to the list | ||
| all_embeddings.extend(el.embedding for el in response.data) | ||
|
|
||
| # Update the meta information only once if it's empty | ||
| if not meta["model"]: | ||
| meta["model"] = response.model | ||
| meta["usage"] = dict(response.usage) | ||
| else: | ||
| # Update the usage tokens | ||
| meta["usage"]["prompt_tokens"] += response.usage.prompt_tokens | ||
| meta["usage"]["total_tokens"] += response.usage.total_tokens | ||
|
|
||
| except Exception as e: | ||
| # Log the error but continue processing | ||
| batch_range = f"{i} - {i + batch_size}" | ||
| logger.exception(f"Failed embedding of documents in range: {batch_range} caused by {e}") | ||
| continue |
There was a problem hiding this comment.
Could you please align this implementation with that of the OpenAIDocumentEmbedder?
I think that it is better for a few reasons:
- groups
argsfor the embedding creation API call - uses the more specific
APIErrorinstead ofException - logs the IDs of the Documents fow which the embedding generation failed
There was a problem hiding this comment.
Thanks for the review @anakin87.
For points 1 and 2, I can update the implementation to:
- Group args for the embedding creation API call
- Use the more specific
APIErrorinstead ofException
For point 3 (logging document IDs), I notice this would require changing the signature of _prepare_texts_to_embed from:
def _prepare_texts_to_embed(self, documents: List[Document]) -> List[str]to:
def _prepare_texts_to_embed(self, documents: List[Document]) -> Dict[str, str]Would you be okay with this signature change to align it with OpenAIDocumentEmbedder's implementation? This would help improve error logging by identifying which specific documents failed during embedding.
There was a problem hiding this comment.
I'm totally OK with changing the signature of _prepare_texts_to_embed.
It's an internal method (_something), so changing its signature and behavior is not considered a breaking change.
releasenotes/notes/add-azure-embedder-exception-handler-c10ea46fb536de3b.yaml
Outdated
Show resolved
Hide resolved
…6fb536de3b.yaml Co-authored-by: Stefano Fiorucci <[email protected]>
Related Issues
Proposed Changes:
How did you test it?
AzureOpenAIDocumentEmbedderclassNotes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.