-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement Integrated Inference #103
Implement Integrated Inference #103
Conversation
…n assert with retries helper function that allows polling and some flexibility for our vector tests
…r integration tests to prevent breakage of specific resources
…ing describe_index_stats
@@ -777,83 +927,6 @@ func (idx *IndexConnection) DeleteAllVectorsInNamespace(ctx context.Context) err | |||
return idx.delete(ctx, &req) | |||
} | |||
|
|||
// [UpdateVectorRequest] holds the parameters for the [IndexConnection.UpdateVector] method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: moved this block up so it lives with the other vector operations
@@ -109,12 +118,12 @@ func (ts *IntegrationTests) TearDownSuite() { | |||
} | |||
|
|||
// Helper funcs | |||
func GenerateTestIndexName() string { | |||
func generateTestIndexName() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: realized all these helpers were being exported out of the package itself, they should be lowercase
@@ -254,6 +264,45 @@ func BuildPodTestIndex(in *Client, name string, tags IndexTags) *Index { | |||
return podIdx | |||
} | |||
|
|||
func retryAssertions(t *testing.T, maxRetries int, delay time.Duration, fn func() error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These helper functions are used in integration tests and are otherwise unrelated to the other changes in this PR. We were seeing a lot of test failures and I needed to add some additional logic to retry specific requests, and poll for freshness in a more targeted way.
… to include sections on integrated inference, and creating integrated indexes
README.md
Outdated
@@ -218,6 +218,53 @@ func main() { | |||
} | |||
``` | |||
|
|||
**Create a serverless integrated index** | |||
|
|||
Integrated inference requires a serverless index configured for a specific embedding model. You can either create a new index for a model, or configure an existing index for a model. To create an index that accepts source text and converts it to vectors automatically using an embedding model hosted by Pinecone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is this comment complete?
To create an index that accepts source text and converts it to vectors automatically using an embedding model hosted by Pinecone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good catch, I think I lopped something off when editing.
golang.org/x/net v0.25.0 // indirect | ||
golang.org/x/sys v0.20.0 // indirect | ||
golang.org/x/text v0.15.0 // indirect | ||
golang.org/x/net v0.33.0 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity, are these changes manual or done as a part of the build process?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean to add context for this in the PR, thanks for flagging.
This was manual, but it was due to PRs that have been put up by dependabot: #99
There was another one opened more recently to bump to 0.36.0, but that will also require bumping the base version of Go specified in the go.mod file, so I wanted to wait for a major release to do that.
@@ -1575,6 +1688,137 @@ indicating higher relevance. | |||
fmt.Printf("rerank response: %+v", rerankResponse) | |||
``` | |||
|
|||
### Integrated Inference | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: not sure if you want to show the example on how to configure an index for a model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I don't think we explicitly show that anywhere else. I can add an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually missed implementing this for configure, thanks again. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, lgtm!!
Problem
The integrated inference API was part of the
2025-01
release, and the code has been generated for Go, but the interface has not been wired up for use in the client.Solution
Implement Integrated Inference:
CreateIndexForModel
toClient
struct.UpsertRecords
andSearchRecords
toIndexConnection
struct.CreateIndexForModelRequest
,CreateIndexForModelEmbed
IntegratedRecord
SearchRecordsRequest
,SearchRecordsQuery
,SearchRecordsRerank
,SearchRecordsVector
Hit
,SearchRecordsResponse
,SearchUsage
Note: I've included some integration test refactoring in this PR. Was running into a lot of flakiness unrelated to this change.
Type of Change
Test Plan
Following example demonstrates creating an integrated index for a specific model, upserting some records, and then searching against those records.