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.

Sign up for GitHub

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

Jump to bottom

Remove AvailabilityForTablet from public API #4706

Closed
keith-turner opened this issue Jun 26, 2024 · 6 comments · Fixed by #4728
Closed

Remove AvailabilityForTablet from public API #4706

keith-turner opened this issue Jun 26, 2024 · 6 comments · Fixed by #4728
Assignees
@meatballspaghetti
Milestone
4.0.0

Comments

@keith-turner
Copy link
Contributor

AvailabilityForTablet is a type in the public API that is currently only used by test code. At one point other parts of the API referenced this, but no longer do. The type should be removed from the public API. The type could possibly be replaced with Pair or Map.Entry.
@keith-turner keith-turner converted this from a draft issue Jun 26, 2024
@ctubbsii
Copy link
Member

I don't think Pair is public API either.

@meatballspaghetti
Copy link
Contributor

I can look into this.

@keith-turner
Copy link
Contributor Author

I don't think Pair is public API either.

Correct, Pair is not public API. Currently this class is only used by test code. The class could be moved out of public API into a test package. Or the class could be deleted and replaced with something more generic because it only has two fields.

@ctubbsii
Copy link
Member

Thanks @keith-turner . I think I misunderstood the ticket to mean that the public API needed to use Pair or Map.Entry. I now understand you meant that the test code that's using it could do that instead of this class.

@keith-turner
Copy link
Contributor Author

I now understand you meant that the test code that's using it could do that instead of this class.

Yeah and I am uncertain about what is best for the test code because I only took a quick glance at it. Not sure of the details of how the test code is using the class. So the suggestion to use Pair or Entry may not be a good one.

meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jul 3, 2024
@meatballspaghetti
Remove AvailabilityForTablet and fix typo
d4c40b3 
- Delete AvailabiltyForTablet class. It is only being used
  in 2 tests: ImportExportIT and TableOperationsIT.
- In both ImportExportIT and TableOperationsIT, replace
  List<AvailabilityForTablet> objects with
  Map<TabletId, TabletAvailability>.
- Fix typo in TableOperationsIT: verifyTabletAvailabilites ->
                                 verifyTabletAvailabilities.

Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jul 3, 2024
@meatballspaghetti
Remove AvailabilityForTablet and fix typo
49d8653 
- Delete AvailabiltyForTablet class. It is only being used
  in 2 tests: ImportExportIT and TableOperationsIT.
- In both ImportExportIT and TableOperationsIT, replace
  List<AvailabilityForTablet> objects with
  Map<TabletId, TabletAvailability>.
- Fix typo in TableOperationsIT: verifyTabletAvailabilites ->
                                 verifyTabletAvailabilities.

Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
@meatballspaghetti meatballspaghetti mentioned this issue Jul 5, 2024
Merged
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jul 8, 2024
@meatballspaghetti
Refractor verifyTabletAvailabilities for simplicity
995c2d1 
- Streamline verifyTabletAvailabilities method in TableOperationsIT
  class by comparing two Maps instead of a Map and a List. Cut
  previous two Assertions down to one Assertion.

Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jul 10, 2024
@meatballspaghetti
Fix formatting in TableOperationsIT
59078fc 
- Correct formatting in TableOperationsIT which was previously
  causing workflow build to fail.

Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
keith-turner pushed a commit that referenced this issue Jul 10, 2024
@meatballspaghetti
Remove AvailabilityForTablet (#4728)
275c309 
- Delete AvailabiltyForTablet class. It is only being used
  in 2 tests: ImportExportIT and TableOperationsIT.
- In both ImportExportIT and TableOperationsIT, replace
  List<AvailabilityForTablet> objects with
  Map<TabletId, TabletAvailability>.
- Fix typo in TableOperationsIT: verifyTabletAvailabilites ->
                                 verifyTabletAvailabilities.

Resolves: #4706 "Remove AvailabilityForTablet from public API"
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
@keith-turner
Copy link
Contributor Author

Completed by #4728

@keith-turner keith-turner linked a pull request Jul 22, 2024 that will close this issue
Remove AvailabilityForTablet #4728
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meatballspaghetti meatballspaghetti

Labels
None yet
Projects
Accumulo: Elasticity
Status: ✅ Done
Milestone
4.0.0
Development

Successfully merging a pull request may close this issue.

Remove AvailabilityForTablet meatballspaghetti/accumulo
3 participants
@keith-turner @ctubbsii @meatballspaghetti