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

Deprecate class FakeThreadPoolMasterService, BlockMasterServiceOnMaster and BusyMasterServiceDisruption, in 'test/framework' directory #4058

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Aug 1, 2022

Description

To support inclusive language, the master terminology is going to be replaced by cluster manager in the code base.

In a previous PR #4051 / commit bea5d1a, 3 classes in test/framework directory that contains master in the name were renamed:

FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnClusterManager
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption

This is a following PR to add back the classes with the old name to keep the backwards compatibility. The classes with the old name will be subclass of the classes with new name, so that maintaining one implementation can support the usage for two classes.

Issues Resolved

A part of issue #3543

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

…er and BusyMasterServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
@tlfeng tlfeng requested review from a team and reta as code owners August 1, 2022 04:30
@tlfeng tlfeng added deprecate v3.0.0 Issues and PRs related to version 3.0.0 backport 2.x Backport to 2.x branch v2.2.0 enhancement Enhancement or improvement to existing feature or request labels Aug 1, 2022
@tlfeng tlfeng changed the title Deprecate class FakeThreadPoolMasterService, BlockMasterServiceOnMaster and BusyMasterServiceDisruption, which in 'test/framework' directory Deprecate class FakeThreadPoolMasterService, BlockMasterServiceOnMaster and BusyMasterServiceDisruption, in 'test/framework' directory Aug 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@tlfeng
Copy link
Collaborator Author

tlfeng commented Aug 1, 2022

In build 1154:

REPRODUCE WITH: ./gradlew ':plugins:repository-s3:yamlRestTest' --tests "org.opensearch.repositories.s3.RepositoryS3ClientYamlTestSuiteIT.test {yaml=repository_s3/20_repository_permanent_credentials/Snapshot and Restore with repository-s3 using permanent credentials}" -Dtests.seed=6E2A5FEA088C4585 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-US -Dtests.timezone=Pacific/Rarotonga -Druntime.java=17 -Dtests.rest.denylist=repository_s3/30_repository_temporary_credentials/*,repository_s3/40_repository_ec2_credentials/*,repository_s3/50_repository_ecs_credentials/*,repository_s3/60_repository_eks_credentials/*

org.opensearch.repositories.s3.RepositoryS3ClientYamlTestSuiteIT > test {yaml=repository_s3/20_repository_permanent_credentials/Snapshot and Restore with repository-s3 using permanent credentials} FAILED
    java.lang.AssertionError: Failure at [repository_s3/20_repository_permanent_credentials:152]: expected [2xx] status code but api [snapshot.create] returned [400 Bad Request] 
[{"error":{"root_cause":[{"type":"invalid_snapshot_name_exception","reason":"[repository_permanent:snapshot-one] Invalid snapshot name [snapshot-one], snapshot with the same name already exists","stack_trace":"InvalidSnapshotNameException[[repository_permanent:snapshot-one] Invalid snapshot name [snapshot-one], snapshot with the same name already exists]
org.opensearch.snapshots.SnapshotsService.ensureSnapshotNameAvailableInRepo(SnapshotsService.java:689)
org.opensearch.snapshots.SnapshotsService$2.execute(SnapshotsService.java:428)
org.opensearch.repositories.blobstore.BlobStoreRepository$1.execute(BlobStoreRepository.java:430)
...

Not related to the code change in this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@tlfeng
Copy link
Collaborator Author

tlfeng commented Aug 1, 2022

@kotwanikunal Thanks a lot for your review! 👍

@tlfeng tlfeng merged commit cebeb8c into opensearch-project:main Aug 1, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 1, 2022
…er and BusyMasterServiceDisruption (#4058)

To support inclusive language, the master terminology is going to be replaced by cluster manager in the code base.

In a previous PR #4051, 3 classes in `test/framework` directory that contains `master` in the name were renamed:
```
FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnClusterManager
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption
```

This is a following commit to add back the classes with the old name to keep the backwards compatibility. The classes with the old name will be subclass of the classes with new name, so that maintaining one implementation can support the usage for two classes.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit cebeb8c)
tlfeng pushed a commit that referenced this pull request Aug 1, 2022
…er and BusyMasterServiceDisruption (#4058) (#4068)

To support inclusive language, the master terminology is going to be replaced by cluster manager in the code base.

In a previous PR #4051 / commit bea5d1a, 3 classes in `test/framework` directory that contains `master` in the name were renamed:
```
FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnClusterManager
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption
```

This is a following commit to add back the classes with the old name to keep the backwards compatibility. The classes with the old name will be subclass of the classes with new name, so that maintaining one implementation can support the usage for two classes.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit cebeb8c)

Co-authored-by: Tianli Feng <[email protected]>
@dreamer-89
Copy link
Member

org.opensearch.repositories.s3.RepositoryS3ClientYamlTestSuiteIT > test {yaml=repository_s3/20_repository_permanent_credentials/Snapshot and Restore with repository-s3 using permanent credentials} FAILED
java.lang.AssertionError: Failure at [repository_s3/20_repository_permanent_credentials:152]: expected [2xx] status code but api [snapshot.create] returned [400 Bad Request]
[{"error":{"root_cause":[{"type":"invalid_snapshot_name_exception","reason":"[repository_permanent:snapshot-one] Invalid snapshot name [snapshot-one], snapshot with the same name already exists","stack_trace":"InvalidSnapshotNameException[[repository_permanent:snapshot-one] Invalid snapshot name [snapshot-one], snapshot with the same name already exists]
org.opensearch.snapshots.SnapshotsService.ensureSnapshotNameAvailableInRepo(SnapshotsService.java:689)
org.opensearch.snapshots.SnapshotsService$2.execute(SnapshotsService.java:428)
org.opensearch.repositories.blobstore.BlobStoreRepository$1.execute(BlobStoreRepository.java:430)

Created issue #4069 to track this

@tlfeng tlfeng deleted the deprecate-master-class-in-test-framework branch August 1, 2022 22:42
pranikum pushed a commit to pranikum/OpenSearch that referenced this pull request Aug 2, 2022
…er and BusyMasterServiceDisruption (opensearch-project#4058)

To support inclusive language, the master terminology is going to be replaced by cluster manager in the code base.

In a previous PR opensearch-project#4051, 3 classes in `test/framework` directory that contains `master` in the name were renamed:
```
FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnClusterManager
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption
```

This is a following commit to add back the classes with the old name to keep the backwards compatibility. The classes with the old name will be subclass of the classes with new name, so that maintaining one implementation can support the usage for two classes.

Signed-off-by: Tianli Feng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch deprecate enhancement Enhancement or improvement to existing feature or request v2.2.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants