Update ConfigureIndex to merge new and previous IndexTags, update README examples
#89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ex to call DescribeIndex and merge tags properly on updates
austin-denoble
commented
Dec 20, 2024
| @@ -90,7 +90,6 @@ func (ts *IntegrationTests) TestDeleteVectorsByFilter() { | |||
|
|
|||
| if ts.indexType == "serverless" { | |||
| assert.Error(ts.T(), err) | |||
| assert.Containsf(ts.T(), err.Error(), "Serverless and Starter indexes do not support deleting with metadata filtering", "Expected error message to contain 'Serverless and Starter indexes do not support deleting with metadata filtering'") | |||
There was a problem hiding this comment.
This error message has changed slightly since this test was originally written. I think it's fine to remove the string check for now and just validate that we've gotten an error back.
austin-denoble
commented
Dec 20, 2024
| @@ -953,8 +960,7 @@ func (c *Client) ConfigureIndex(ctx context.Context, name string, in ConfigureIn | |||
| } | |||
| } | |||
| request.DeletionProtection = (*db_control.DeletionProtection)(deletionProtection) | |||
| request.Tags = (*db_control.IndexTags)(&in.Tags) | |||
| fmt.Printf("request.Tags: %+v\n", request.Tags) | |||
There was a problem hiding this comment.
whoops - left this in previously
haruska
approved these changes
Dec 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
@jhamon called out that since
ConfigureIndexis a PATCH operation, we should probably be explicitly merging existing tags with incoming tags when configuring an index as a partial update. See here for a python example: https://github.com/pinecone-io/pinecone-python-client/pull/426/files#diff-9e7f01373e4589c7e4ded21935023cdf187ab6ce8b2b44a0d5470a27eaf464eeR303-R314Solution
ConfigureIndexto callDescribeIndexand merge tags properly on updates.READMEto add examples of usingIndexTagswhen creating and updating indexes.Type of Change
Test Plan
just testMake sure CI is green.