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.

Sign up for GitHub

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

Jump to bottom

Java: `FT.CREATE` #2414

Merged

Yury-Fridlyand merged 5 commits into release-1.2 from java/yuryf-ft-create

Oct 11, 2024

Collaborator

Yury-Fridlyand commented Oct 9, 2024 •

edited

Loading

Issue link

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.

Yury-Fridlyand added the java label

Yury-Fridlyand requested a review from a team as a code owner

October 9, 2024 17:44

prateek-kumar-improving reviewed

View reviewed changes

java/client/src/main/java/glide/api/commands/servermodules/FT.java Outdated Show resolved Hide resolved


          FT.CREATE

c9d9fa1

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand force-pushed the java/yuryf-ft-create branch from 113a983 to c9d9fa1 Compare

October 9, 2024 18:20

Yury-Fridlyand mentioned this pull request

Node: VSS commands support - Preliminary Commands #738

Closed

2 tasks

acarbonetto reviewed

View reviewed changes

.github/workflows/java.yml Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/commands/servermodules/FT.java Show resolved Hide resolved

java/client/src/main/java/glide/api/commands/servermodules/FT.java Show resolved Hide resolved

java/client/src/main/java/glide/api/commands/servermodules/FT.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated

+                  /** Type of the index dataset. */
+                  public enum IndexType {
+                      /** Data stored in hashes, so field identifiers are field names within the hashes. */
+                      HASH,

Contributor

acarbonetto Oct 9, 2024

nit

Suggested change

      
                    HASH,
          
                    HASH("ON HASH"),

Collaborator Author

Yury-Fridlyand Oct 9, 2024

This should be 2 different args

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated

+                  public static class NumericField implements Field {
+                      @Override
+                      public String[] toArgs() {
+                          return new String[] {FieldType.NUMERIC.toString()};

Contributor

acarbonetto Oct 9, 2024

why use an enum here? a String works just as well

Collaborator Author

Yury-Fridlyand Oct 9, 2024

A recommendation from @prateek-kumar-improving to use constants. Could be helpful when we use it in multiple places

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated

+                      private Optional<Character> separator;
+                      private final boolean caseSensitive;
+                      /** Create a <code>TAG</code> field. */

Contributor

acarbonetto Oct 9, 2024

warning here? Should only be used for HASH index types?

Collaborator Author

Yury-Fridlyand Oct 9, 2024

Why? Can't find such thing in docs.

java/client/src/main/java/glide/api/models/commands/vss/FTCreateOptions.java Outdated

+                       *
+                       * @param caseSensitive Whether to keep the original case.
+                       */
+                      public TagField(boolean caseSensitive) {

Contributor

acarbonetto Oct 9, 2024

warning here? Should only be used for HASH index types?

Collaborator

prateek-kumar-improving Oct 10, 2024

Could not find this in the documentation?


          Address PR review.

cd9ceef

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand temporarily deployed to AWS_ACTIONS

October 9, 2024 20:14

— with

GitHub Actions Inactive

This was referenced Oct 9, 2024

Java: add FT.INFO #2405

Merged

Java: VSS commands init #2385

Closed

prateek-kumar-improving approved these changes

View reviewed changes

acarbonetto reviewed

View reviewed changes

java/client/src/main/java/glide/api/commands/servermodules/FT.java Show resolved Hide resolved

acarbonetto reviewed

View reviewed changes

java/client/src/main/java/glide/api/commands/servermodules/FT.java Show resolved Hide resolved


          docs

6aece71

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand temporarily deployed to AWS_ACTIONS

October 10, 2024 17:16

— with

GitHub Actions Inactive

Yury-Fridlyand changed the base branch from main to release-1.2

October 10, 2024 17:35


          glidestring

707a079

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand had a problem deploying to AWS_ACTIONS

October 10, 2024 18:59

— with

GitHub Actions Error

acarbonetto mentioned this pull request

Java: VSS commands support #2428

Closed

13 tasks

jamesx-improving reviewed

View reviewed changes

java/client/src/main/java/glide/api/commands/servermodules/FT.java

+                          @NonNull String indexName,
+                          @NonNull FieldInfo[] fields,
+                          @NonNull FTCreateOptions options) {
+                      return create(client, gs(indexName), fields, options);

Collaborator

jamesx-improving Oct 11, 2024

Will accepting String parameter (indexName here) but passing the GlideString version to customCommand() affect the output of the command anyhow?

Collaborator Author

Yury-Fridlyand Oct 11, 2024

There is a binary version of custom command is used. I hope no.

jamesx-improving reviewed

View reviewed changes

java/integTest/src/test/java/glide/modules/VectorSearchTests.java

                   @SneakyThrows
-                  public void check_module_loaded() {
-                      var client =
+                  public static void init() {

Collaborator

jamesx-improving Oct 11, 2024

Question: is this approach of creating a single client and share it among all tests preferred over creating a new client for every tests?

Collaborator Author

Yury-Fridlyand Oct 11, 2024

There is no preference so far. Do whatever you like in your tests.

acarbonetto approved these changes

View reviewed changes

Contributor

acarbonetto left a comment

some small comments - please address before merging

java/client/src/main/java/glide/api/models/commands/FT/FTCreateOptions.java Show resolved Hide resolved

java/client/src/main/java/glide/api/models/commands/FT/FTCreateOptions.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/models/commands/FT/FTCreateOptions.java Outdated Show resolved Hide resolved

java/client/src/main/java/module-info.java

@@ @@ -9,8 +9,10 @@ @@
                   exports glide.api.models.commands.function;
                   exports glide.api.models.commands.scan;
                   exports glide.api.models.commands.stream;
+                  exports glide.api.models.commands.FT;

Contributor

acarbonetto Oct 11, 2024

⭐

java/integTest/src/test/java/glide/modules/VectorSearchTests.java Outdated Show resolved Hide resolved

prateek-kumar-improving reviewed

View reviewed changes

java/client/src/main/java/glide/api/models/commands/FT/FTCreateOptions.java Show resolved Hide resolved


          Address PR review.

a9669a2

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand temporarily deployed to AWS_ACTIONS

October 11, 2024 17:21

— with

GitHub Actions Inactive

Yury-Fridlyand merged commit bbfce44 into release-1.2

8 checks passed

Yury-Fridlyand deleted the java/yuryf-ft-create branch

October 11, 2024 18:08

Yury-Fridlyand added a commit that referenced this pull request


          Java: FT.CREATE (#2414)

08db437

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand mentioned this pull request

Java: FT.CREATE #2438

Closed

prateek-kumar-improving mentioned this pull request

Python: VSS commands support #2429

Closed

14 tasks

Muhammad-awawdi-amazon pushed a commit that referenced this pull request


          Java: FT.CREATE (#2414)

18f2f46

Signed-off-by: Yury-Fridlyand <[email protected]>

Muhammad-awawdi-amazon pushed a commit to Muhammad-awawdi-amazon/valkey-glide that referenced this pull request


          Java: FT.CREATE (valkey-io#2414)

c27f194

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Muhammad Awawdi <[email protected]>

avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request


          Java: FT.CREATE (valkey-io#2414)

9801bc7

Signed-off-by: Yury-Fridlyand <[email protected]>

avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request


          Java: FT.CREATE (valkey-io#2414)

599c530

Signed-off-by: Yury-Fridlyand <[email protected]>

Muhammad-awawdi-amazon pushed a commit to Muhammad-awawdi-amazon/valkey-glide that referenced this pull request


          Java: FT.CREATE (valkey-io#2414)

be2fe5b

Signed-off-by: Yury-Fridlyand <[email protected]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java