Skip to content
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

Update Marqo Document API Response Format #911

Merged
merged 46 commits into from
Aug 2, 2024
Merged

Conversation

wanliAlex
Copy link
Collaborator

@wanliAlex wanliAlex commented Jul 19, 2024

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix

  • What is the current behavior? (You can also link to an open issue here)
    When we feed a batch of documents to Vespa, if there is an http error (e.g., time out error) for one document, the request will raise a 500 error without the status of each document. This could be a transient network issue that only happens to one document while other documents are indexed into Vespa. We modify the error message logic to better demonstrate the indexing results.

  • What is the new behavior (if this is a feature change)?
    In this PR, we update the Marqo document API format. Each document will have its individual response status for batch requests (update documents, add documents, get_documents_by_ids). For example, if a single document fails due to a transient network issue, we still return 200 for the whole batch response, but 500 for the specific document.

We also include the failure count, error count, and success count in the returned response's header for SLA purpose.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    no

  • Have unit tests been run against this PR? (Has there also been any additional testing?)

  • Related Python client changes (link commit/PR here)

  • Related documentation changes (link commit/PR here)

  • Other information:

  • Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added / updated (for bug fixes / features)

src/marqo/vespa/models/__init__.py Outdated Show resolved Hide resolved
src/marqo/vespa/models/feed_response.py Show resolved Hide resolved
src/marqo/vespa/vespa_client.py Outdated Show resolved Hide resolved
src/marqo/vespa/vespa_client.py Outdated Show resolved Hide resolved
src/marqo/vespa/vespa_client.py Outdated Show resolved Hide resolved
src/marqo/vespa/vespa_client.py Outdated Show resolved Hide resolved
src/marqo/vespa/vespa_client.py Outdated Show resolved Hide resolved
papa99do
papa99do previously approved these changes Aug 2, 2024
@wanliAlex wanliAlex merged commit 4345921 into mainline Aug 2, 2024
8 checks passed
@wanliAlex wanliAlex deleted the li/patch-batch branch August 2, 2024 05:49
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