-
Notifications
You must be signed in to change notification settings - Fork 282
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
Upgrade to OpenSearch 2.0.0 #1698
Upgrade to OpenSearch 2.0.0 #1698
Conversation
bb831fe
to
054d29d
Compare
@cliu123 Can you rebase? There were some recent version number simplifications that would make sure the plugin install test would use the same OpenSearch version as the build. |
881963e
to
d3966e7
Compare
69bdefe
to
8d13900
Compare
Codecov Report
@@ Coverage Diff @@
## main #1698 +/- ##
============================================
- Coverage 62.88% 60.38% -2.50%
+ Complexity 3264 3195 -69
============================================
Files 253 253
Lines 18102 18095 -7
Branches 3247 3245 -2
============================================
- Hits 11383 10927 -456
- Misses 5064 5586 +522
+ Partials 1655 1582 -73
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback on the changes
id = _id; | ||
|
||
try { | ||
ConfigHelper.fromYamlFile(filepath, CType.fromString(_id), 1, 0, 0); | ||
ConfigHelper.fromYamlFile(filepath, CType.fromString(_id), 2, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean for this value to switch from 1 -> 2? Additional documentation would be useful in this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was for migration Config 6 -> Config 7. But OpenSearch doesn't need to support this migration. Removing the 2 test classes for the same reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we include documentation in the code as to how this value is determine or aligned to. This could also be done with a local variable eg. final int latestSupportedMajorVersion = 2;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea! Can we do that in a separate PR? Let's unblock 2.0.0 build first.
8d13900
to
a41afd8
Compare
a41afd8
to
b18b727
Compare
b18b727
to
2c54fc0
Compare
Signed-off-by: cliu123 <lc12251109@gmail.com>
1f91d34
to
c28ff0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note; if we are moving away from gradle.properties
we should delete it
e89f56e
to
3bb7943
Compare
6db9676
to
89badc7
Compare
@@ -173,7 +173,7 @@ public void onResponse(AcknowledgedResponse response) { | |||
public void onResponse(CreateIndexResponse response) { | |||
final List<SecurityDynamicConfiguration<?>> dynamicConfigurations = builder.build(); | |||
final ImmutableList.Builder<String> cTypes = ImmutableList.builderWithExpectedSize(dynamicConfigurations.size()); | |||
final BulkRequestBuilder br = client.prepareBulk(opendistroIndex, "_doc"); | |||
final BulkRequestBuilder br = client.prepareBulk(opendistroIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity, why is this change reqd?
Signed-off-by: cliu123 <lc12251109@gmail.com>
Description
[Describe what this change achieves]
Issues Resolved
#1642
Testing
UT
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.