Allow setting default dialect for search module #3452
Conversation
sazzad16
added
breakingchange
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #3452 +/- ##
=========================================
Coverage 71.32% 71.33%
- Complexity 4739 4750 +11
=========================================
Files 274 274
Lines 14697 14716 +19
Branches 991 992 +1
=========================================
+ Hits 10483 10497 +14
- Misses 3785 3786 +1
- Partials 429 433 +4
☔ View full report in Codecov by Sentry. |
|
@sazzad16 Could you add the same for FT.AGGREGATE? I've updated the issue description. |
sazzad16
changed the title
| @@ -4168,6 +4172,11 @@ public void setJsonObjectMapper(JsonObjectMapper jsonObjectMapper) { | |||
| this.jsonObjectMapper = jsonObjectMapper; | |||
| } | |||
|
|
|||
| public void setDefaultSearchDialect(int dialect) { | |||
There was a problem hiding this comment.
The issue says This configuration option should be optional. If it is not specified, the client should fall back to the current behavior.
Raising this argument isn't that.
There was a problem hiding this comment.
Firstly, users would be using the one in UnifiedJedis. I guess we're concerned about that and so check my answer in test file.
| @Override | ||
| public void setUp() { | ||
| super.setUp(); | ||
| client.setDefaultSearchDialect(DEFAULT_DIALECT); |
There was a problem hiding this comment.
@chayim Here, I'm setting a default dialect for client apis. This is set once and used in all the following tests.
More importantly, assuming you're conerned about thsi, IT IS OPTIONAL. For example, this configuration is NOT SET in other search test files.
Resolves #3447