-
Notifications
You must be signed in to change notification settings - Fork 40
Allow embed
as a parameter to configureIndex
#338
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
base: main
Are you sure you want to change the base?
Conversation
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.
LGTM!
@@ -37,7 +37,7 @@ describe('configureIndex argument validations', () => { | |||
|
|||
await expect(toThrow).rejects.toThrowError(PineconeArgumentError); | |||
await expect(toThrow).rejects.toThrowError( | |||
'Object contained invalid properties: speculoos. Valid properties include deletionProtection, spec, tags.' | |||
'Object contained invalid properties: speculoos. Valid properties include deletionProtection, spec, tags, embed.' |
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.
speculoos? lol
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.
Yeah, it's the invalid object key which ends up in the error message.
Problem
Currently, the
ConfigureIndexRequest
interface supportsembed?: ConfigureIndexRequestEmbed
as a parameter, which aligns with the OAS spec and how the endpoint can be used:pinecone-ts-client/src/pinecone-generated-ts-fetch/db_control/models/ConfigureIndexRequest.ts
Line 64 in e5b39d7
However, the
embed
field was not added toConfigureIndexRequestProperties
, defining what we expect a user to provide in a request. We also error out if the user attempts to configure an index with just theembed
property.Solution
Update
ConfigureIndexRequestProperties
and the validation logic in theconfigureIndex
action to allow leveragingembed
properly.Type of Change\
Test Plan
CI Unit & Integration Tests