-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Refactor RefreshStats with Builder pattern #19835
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
Refactor RefreshStats with Builder pattern #19835
Conversation
|
❌ Gradle check result for 1e1b027: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 1e1b027: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 1e1b027: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
sandeshkr419
left a comment
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.
Minor correction on deprecation version, rest LGTM.
Thanks for refactoring this @pado0!
server/src/main/java/org/opensearch/index/refresh/RefreshStats.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Jean Kim <[email protected]>
Signed-off-by: Jean Kim <[email protected]>
Signed-off-by: Jean Kim <[email protected]>
c48eedb to
6bf8163
Compare
Signed-off-by: Jean Kim <[email protected]>
6bf8163 to
c128664
Compare
|
❕ Gradle check result for c128664: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19835 +/- ##
============================================
- Coverage 73.09% 73.02% -0.08%
+ Complexity 71060 71010 -50
============================================
Files 5754 5754
Lines 325230 325295 +65
Branches 47035 47036 +1
============================================
- Hits 237735 237547 -188
- Misses 68359 68630 +271
+ Partials 19136 19118 -18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Sandesh Kumar <[email protected]>
Co-authored-by: Sandesh Kumar <[email protected]>
Description
This PR refactors the
RefreshStatsclass to use the Builder pattern instead of relying on multiple constructors.By adopting the Builder pattern, it becomes easier to evolve the stats API, add new metrics, and maintain backward compatibility without forcing disruptive constructor changes.
Based on the related issue:
There are multiple stats-related classes that need similar refactoring, and we are addressing them in priority order. This PR covers RefreshStats as part of that effort.
Related Issues
Related to #19225
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.