Skip to content

Node: add FT.CREATE command#2501

Merged
acarbonetto merged 27 commits intorelease-1.2from
node/acarbo_add_ft_create
Oct 23, 2024
Merged

Node: add FT.CREATE command#2501
acarbonetto merged 27 commits intorelease-1.2from
node/acarbo_add_ft_create

Conversation

@acarbonetto
Copy link
Copy Markdown
Contributor

@acarbonetto acarbonetto commented Oct 22, 2024

Issue link

This Pull Request is linked to issue (URL): #738

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Commits will be squashed upon merging.

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
expect(
await GlideFt.create(client, uuidv4(), [vectorField_3], {
dataType: "HASH",
prefixes: ["docs:"],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When you'll test search or aggregate, use {} in prefix to ensure same slot

@Yury-Fridlyand Yury-Fridlyand added the node 🐢 Node.js wrapper label Oct 22, 2024
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
"fix-protobuf-file": "replace 'this\\.encode\\(message, writer\\)\\.ldelim' 'this.encode(message, writer && writer.len ? writer.fork() : writer).ldelim' src/ProtobufMessage.js",
"test": "npm run build-test-utils && jest --verbose --runInBand --testPathIgnorePatterns='ServerModules'",
"test-modules": "npm run build-test-utils && jest --verbose --runInBand --testPathPattern='ServerModules'",
"test-minimum": "npm run build-test-utils && jest --verbose --runInBand --testNamePattern='^(.(?!(GlideJson|GlideFt|pubsub|kill)))*$'",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

small nit: with a negative lookahead removing the modules/pubsub/kill tests, would it be better to name this something like base rather than minimum?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't like base. I originally called it 'fast'. The idea is that it skips the long tests, and could maybe be used for doing a sanity check on the client.

});

describe("GlideFt", () => {
let cluster: ValkeyCluster;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think you can reuse this instead of copy-pasting. Either move common code to a helper function or merge top-level describe blocks (you can define nested blocks inside).

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
@acarbonetto acarbonetto merged commit 78c927b into release-1.2 Oct 23, 2024
@acarbonetto acarbonetto deleted the node/acarbo_add_ft_create branch October 23, 2024 15:54
cyip10 pushed a commit that referenced this pull request Oct 23, 2024
* Add FT.CREATE command for Node

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
prateek-kumar-improving pushed a commit that referenced this pull request Oct 25, 2024
* Add FT.CREATE command for Node

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
BoazBD pushed a commit to BoazBD/valkey-glide that referenced this pull request Oct 27, 2024
* Add FT.CREATE command for Node

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: BoazBD <boazbd@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

node 🐢 Node.js wrapper

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants