Skip to content
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

changed usages of "master" to "clusterManager" in variable names #504

Merged

Conversation

amitgalitz
Copy link
Member

Signed-off-by: Amit Galitzky amgalitz@amazon.com

Description

This is our first PR in changing the naming in AD to be more inclusive. Most of the comments and variables names in AD where master was used is now changed to lead. Other method usages and dependency haven't changed yet since they still haven't changed in Opensearch core. Once those are changed then we will also make the appropriate changes.

Check List

  • Commits are signed per the DCO using --signoff

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.

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
@amitgalitz amitgalitz added enhancement New feature or request v2.0.0 labels Apr 15, 2022
@amitgalitz amitgalitz requested a review from a team April 15, 2022 22:19
@ohltyler
Copy link
Member

Why not change to cluster_manager for consistency with core? Ref: #397

@amitgalitz
Copy link
Member Author

Why not change to cluster_manager for consistency with core? Ref: #397

Fair point, I changed it first to lead cause I originally saw this issue opensearch-project/OpenSearch#472 that mentions changing to "leader", I later made the change for cluster_manager on one of the imports and hence realizing that core changed to cluster_manager on some occasions.

@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2022

Codecov Report

Merging #504 (9030770) into main (b5bebb2) will increase coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #504      +/-   ##
============================================
+ Coverage     78.93%   78.95%   +0.02%     
  Complexity     4193     4193              
============================================
  Files           296      296              
  Lines         17663    17663              
  Branches       1878     1878              
============================================
+ Hits          13942    13946       +4     
+ Misses         2820     2816       -4     
  Partials        901      901              
Flag Coverage Δ
plugin 78.95% <50.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../main/java/org/opensearch/ad/cluster/HashRing.java 81.37% <ø> (ø)
...a/org/opensearch/ad/cluster/LeadEventListener.java 94.59% <ø> (ø)
...opensearch/ad/indices/AnomalyDetectionIndices.java 72.12% <0.00%> (ø)
...pensearch/ad/settings/AnomalyDetectorSettings.java 100.00% <ø> (ø)
.../java/org/opensearch/ad/AnomalyDetectorPlugin.java 96.53% <100.00%> (ø)
...ava/org/opensearch/ad/task/ADHCBatchTaskCache.java 88.88% <0.00%> (-1.24%) ⬇️
...ain/java/org/opensearch/ad/task/ADTaskManager.java 76.90% <0.00%> (+0.22%) ⬆️
.../main/java/org/opensearch/ad/ml/CheckpointDao.java 70.19% <0.00%> (+0.64%) ⬆️

@amitgalitz
Copy link
Member Author

Why not change to cluster_manager for consistency with core? Ref: #397

Fair point, I changed it first to lead cause I originally saw this issue opensearch-project/OpenSearch#472 that mentions changing to "leader", I later made the change for cluster_manager on one of the imports and hence realizing that core changed to cluster_manager on some occasions.

will change to clusterManager

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
@ohltyler ohltyler changed the title changed usages of "master" to "lead" in variable names changed usages of "master" to "clusterManager" in variable names Apr 18, 2022
Copy link
Member

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@amitgalitz amitgalitz merged commit fc1a6e2 into opensearch-project:main Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants