Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

feat(NODE-6044): add detailed types for SearchIndexDescription #4052

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

Open
noseworthy wants to merge 1 commit into mongodb:main
base: main
Choose a base branch
Loading
from noseworthy:add-search-index-types

Conversation

Copy link

@noseworthy noseworthy commented Mar 25, 2024

Description

Adds more detailed type definitions for SearchIndexDescription and SearchIndexDefinition based on the documentation found at Create an Atlas Search Index.

The existing type:

export interface SearchIndexDescription {
 name?: string;
 definition: Document;
}

allows users to use any object for the definition property. Search index definitions are fairly complicated and having more strict types defined for this property will make it easier to use the createSearchIndex() and updateSearchIndex() apis.

What is changing?

Added full types for defining a search index definition.

Is there new documentation needed for these changes?

I don't think any new hand-written documentation is necessary, but new tsdoc documentation should be generated to capture the new types.

What is the motivation for this change?

Using the createSearchIndex() and updateSearchIndex() apis when the definition is just specified to be a bson.Document is a bit unwieldy. The search index definitions can be fairly complicated, and it's very helpful to have TypeScript help validate the definitions.

Release Highlight

Fill in title or leave empty for no highlight

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

chadwickellis reacted with thumbs up emoji
Copy link
Contributor

Hi @noseworthy thanks so much for all your work on this PR. We're currently reviewing this and will get back to you soon.
noseworthy reacted with thumbs up emoji

Copy link
Contributor

Hi @noseworthy! Thanks for your patience and thanks again for all your hard work on this PR. After reviewing it with the team, we've decided that accepting these changes into the driver isn't the right choice for us at the moment. Taking on the burden of keeping these types accurate as the Atlas Search product continues to be developed is not something we can commit to at this time.
The driver also might not be the right place for these changes as the type definitions would then be bound to specific releases of the Node driver. This is something we generally try to avoid as we put a lot of work into having our driver be compatible and easy to use across all our currently supported server versions.
We will leave this PR open for consideration later down the line.

@nbbeeken nbbeeken added External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system labels Mar 28, 2024
Copy link
Author

Hi @noseworthy! Thanks for your patience and thanks again for all your hard work on this PR. After reviewing it with the team, we've decided that accepting these changes into the driver isn't the right choice for us at the moment. Taking on the burden of keeping these types accurate as the Atlas Search product continues to be developed is not something we can commit to at this time. The driver also might not be the right place for these changes as the type definitions would then be bound to specific releases of the Node driver. This is something we generally try to avoid as we put a lot of work into having our driver be compatible and easy to use across all our currently supported server versions. We will leave this PR open for consideration later down the line.

No worries, @W-A-James . Thanks for taking a look and considering it. We're really excited to start using the Atlas Search features more now that we can run it locally for our dev environments but I found definitions for the indices really complicated. I was hoping that this would've been able to make it a bit more approachable than a simple bson.Document declaration. But I totally understand the burden of maintaining these types. Like I said, they're complicated and I'm sure with vector search changing frequently too it'd be a huge pain.

Hopefully as the product evolves we can get better typescript support for some of these features 🤞. Thanks for all the team's work on the driver!
nbbeeken, W-A-James, and alexbevi reacted with heart emoji

@W-A-James W-A-James removed their assignment Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /