Node: Add `FT.PROFILE` command by acarbonetto · Pull Request #2633 · valkey-io/valkey-glide

acarbonetto · 2024-11-06T23:26:09Z

Issue link

This Pull Request is linked to issue (URL): #2527
depends on: #2554

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.

Passing tests on 599a828

node/src/server-modules/GlideFt.ts

node/tests/ServerModules.test.ts

node/src/server-modules/GlideFt.ts

acarbonetto · 2024-11-08T08:07:37Z

node/tests/ServerModules.test.ts


        it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
-            "FT.AGGREGATE ft.aggregate",
+            "FT.INFO ft.info",


moved from below

i mightve missed the context but why is this the only test with upper and lower case descriptions?

I don't know how jest applies test filter (case sensitive or not) so I duplicated the test names

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

Yury-Fridlyand · 2024-11-12T21:54:54Z

node/tests/ServerModules.test.ts

+                const aggregProfile: [
+                    FtAggregateReturnType,
+                    Record<string, number>,
+                ] = await GlideFt.profileAggregate(


I think you can omit type there

Suggested change

const aggregProfile: [

FtAggregateReturnType,

Record<string, number>,

] = await GlideFt.profileAggregate(

const aggregProfile = await GlideFt.profileAggregate(

I prefer to add the type.

Yury-Fridlyand · 2024-11-12T21:55:28Z

node/tests/ServerModules.test.ts

+                        .sort((a, b) => (a["genre"]! > b["genre"]! ? 1 : -1)),
+                ).toEqual(expectedAggreg);
+
+                const aggregProfile: [


so same here

Yury-Fridlyand · 2024-11-12T21:55:37Z

node/tests/ServerModules.test.ts

+                expect(binaryResult).toEqual(expectedBinaryResult);

-                response = await GlideFt.info(client, index, {
+                const binaryProfileResult: [


Yury-Fridlyand · 2024-11-12T21:55:43Z

node/tests/ServerModules.test.ts

+                ];
+                expect(stringResult).toEqual(expectedStringResult);
+
+                const stringProfileResult: [


* Node: Add FT.PROFILE command --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Node: Add FT.PROFILE command --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Muhammad Awawdi <mawawdi@amazon.com>

* Node: Add FT.PROFILE command --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Node: Add FT.PROFILE command --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Muhammad Awawdi <mawawdi@amazon.com>

* Node: Add FT.PROFILE command --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>

acarbonetto requested a review from a team as a code owner November 6, 2024 23:26

acarbonetto had a problem deploying to AWS_ACTIONS November 6, 2024 23:26 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 6, 2024 23:28 — with GitHub Actions Error

Yury-Fridlyand reviewed Nov 6, 2024

View reviewed changes

acarbonetto had a problem deploying to AWS_ACTIONS November 7, 2024 00:10 — with GitHub Actions Failure

Yury-Fridlyand reviewed Nov 7, 2024

View reviewed changes

node/src/server-modules/GlideFt.ts Outdated Show resolved Hide resolved

node/src/server-modules/GlideFt.ts Outdated Show resolved Hide resolved

Yury-Fridlyand added the node 🐢 Node.js wrapper label Nov 8, 2024

Yury-Fridlyand mentioned this pull request Nov 8, 2024

Node: VSS Commands support - aggregate commands (part II) #2527

Closed

6 tasks

acarbonetto force-pushed the node/acarbo_add_ft_profile branch from eb7e086 to b498c6c Compare November 8, 2024 06:04

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 06:04 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 06:28 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 06:35 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 06:48 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 06:55 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 07:12 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 07:21 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 07:57 — with GitHub Actions Error

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 08:03 — with GitHub Actions Error

acarbonetto temporarily deployed to AWS_ACTIONS November 8, 2024 08:05 — with GitHub Actions Inactive

acarbonetto had a problem deploying to AWS_ACTIONS November 8, 2024 08:05 — with GitHub Actions Error

acarbonetto temporarily deployed to AWS_ACTIONS November 8, 2024 08:05 — with GitHub Actions Inactive

acarbonetto commented Nov 8, 2024

View reviewed changes

Remove duplicate interfaces

85add91

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

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:08 — with GitHub Actions Error

Re-add merge conflicted test

7088401

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

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:12 — with GitHub Actions Error

Update tests

92d1387

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

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:21 — with GitHub Actions Error

prettier

a814baf

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

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:24 — with GitHub Actions Error

better tests

599a828

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

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:34 — with GitHub Actions Failure

acarbonetto temporarily deployed to AWS_ACTIONS November 12, 2024 21:34 — with GitHub Actions Inactive

acarbonetto had a problem deploying to AWS_ACTIONS November 12, 2024 21:34 — with GitHub Actions Failure

Yury-Fridlyand approved these changes Nov 12, 2024

View reviewed changes

acarbonetto merged commit c374d29 into release-1.2 Nov 12, 2024

acarbonetto deleted the node/acarbo_add_ft_profile branch November 12, 2024 23:22

Conversation

acarbonetto commented Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue link

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

acarbonetto commented Nov 6, 2024 •

edited

Loading