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

feat: Allow boolean returnMetadata for Vectorize V2 queries #2596

Conversation

garvit-gupta
Copy link
Contributor

Allow boolean returnMetadata for Vectorize V2 queries for backward compatibility with Vectorize V1.

Comment on lines 243 to 246
): value is VectorizeMetadataRetrievalLevel {
): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for this change? I think it makes sense to keep it as it was

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to get rid of a warning Invalid type "never" of template literal expression.eslint[@typescript-eslint/restrict-template-expressions](https://typescript-eslint.io/rules/restrict-template-expressions)

Copy link
Contributor

@RamIdeas RamIdeas left a comment

Choose a reason for hiding this comment

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

You'll need to run prettier on vectorize-api.ts

@garvit-gupta garvit-gupta force-pushed the garvit/NO-JIRA/vectorize-return-metadata-type branch from 3fbb9bf to fced277 Compare August 23, 2024 13:33
@garvit-gupta garvit-gupta marked this pull request as ready for review August 23, 2024 13:34
@garvit-gupta garvit-gupta requested review from a team as code owners August 23, 2024 13:34
);
if (options && options.returnMetadata) {
if (
typeof options.returnMetadata !== 'boolean' &&
Copy link
Member

Choose a reason for hiding this comment

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

Maybe perform this type check once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you recommend this to be refactored?

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

The checks could be made a bit more efficient but overall LGTM

Co-authored-by: James M Snell <jasnell@gmail.com>
@fhanau fhanau merged commit 1739783 into cloudflare:main Aug 30, 2024
10 checks passed
@garvit-gupta garvit-gupta deleted the garvit/NO-JIRA/vectorize-return-metadata-type branch September 3, 2024 12:32
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.

5 participants