Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

Delete documents starter env #51

Merged
merged 13 commits into from
Oct 2, 2023
Merged

Delete documents starter env #51

merged 13 commits into from
Oct 2, 2023

Conversation

acatav
Copy link
Contributor

@acatav acatav commented Sep 28, 2023

Problem

Starter env do not support delete by metadata

Solution

Currently don't delete on upsert, and for explicit delete documents delete first 50 chunks

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Added tests for large upserts and delete

@acatav acatav changed the base branch from main to dev September 28, 2023 13:54
Copy link
Contributor

@miararoy miararoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a minor stuff

resin/knoweldge_base/base.py Show resolved Hide resolved
resin/knoweldge_base/knowledge_base.py Outdated Show resolved Hide resolved
resin/knoweldge_base/knowledge_base.py Outdated Show resolved Hide resolved
resin/knoweldge_base/knowledge_base.py Outdated Show resolved Hide resolved
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acatav please see a few comments

resin/knoweldge_base/knowledge_base.py Outdated Show resolved Hide resolved
resin/knoweldge_base/knowledge_base.py Outdated Show resolved Hide resolved
tests/system/knowledge_base/test_knowledge_base.py Outdated Show resolved Hide resolved
tests/system/knowledge_base/test_knowledge_base.py Outdated Show resolved Hide resolved
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@acatav acatav requested a review from miararoy October 2, 2023 08:12
Copy link
Contributor

@miararoy miararoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@acatav acatav merged commit b17bf0c into dev Oct 2, 2023
@acatav acatav deleted the delete-documents-starter-env branch October 2, 2023 08:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants