Skip to content

Remove interconnected IMA, Thrift, and ClientImpl non-public methods#3253

Open
SethSmucker wants to merge 26 commits into
integrationfrom
task/ima-remove-the-rest
Open

Remove interconnected IMA, Thrift, and ClientImpl non-public methods#3253
SethSmucker wants to merge 26 commits into
integrationfrom
task/ima-remove-the-rest

Conversation

@SethSmucker
Copy link
Copy Markdown
Collaborator

@SethSmucker SethSmucker commented Oct 29, 2025

InMemoryAccumulo, Thrift, and ClientImpl are non-public methods in Accumulo 2.1. All three needed to be removed in a group since they were reliant on each other. They have been replaced with public variations where applicable, or new methods have been created to handle the job they used to do.

Part of #2443

@SethSmucker SethSmucker changed the title Task/ima remove the rest Remove interconnected IMA, Thrift, and ClientImpl non-public methods Oct 29, 2025
@SethSmucker SethSmucker marked this pull request as ready for review December 8, 2025 21:18
@SethSmucker SethSmucker requested review from apmoriarty, dlmarion, ivakegg and lbschanno and removed request for dlmarion December 8, 2025 21:19
- Remove extends ClientContext from InMemoryAccumuloClient
- Remove extends Connector from InMemoryConnector
- Remove implements Instance from InMemoryInstance
- Delete InMemoryClientInfo.java (used non-public Credentials/ClientInfoImpl)
- Clean up BulkInputFormat TabletLocator usage
- Update PushdownScheduler to remove ClientContext usage

Part of #2443
@SethSmucker
Copy link
Copy Markdown
Collaborator Author

Stacked PR Notice

This PR is the BASE of a stacked PR chain. The following PRs depend on this one and share files:

PR Title Status
#3253 Remove interconnected IMA, Thrift, and ClientImpl THIS PR (BASE)
#3345 Replace InMemoryInstance with InMemoryAccumulo Stacked on this
#3227 Remove non-public clientImpl.* usages DRAFT, stacked on this

Shared files:

  • InMemoryAccumuloClient.java
  • InMemoryClientInfo.java
  • InMemoryConnector.java
  • InMemoryInstance.java
  • BulkInputFormat.java

IMPORTANT: This PR must merge FIRST. After merging, #3345 and #3227 will need to be updated with integration.

Apply formatter, sortpom, and impsort fixes to pass CI check.
The getClientContext method used non-public ClientContext API and was
only called from PushdownScheduler. Since PushdownScheduler now uses
the public tableOperations().tableIdMap() API instead, this utility
method is no longer needed.

Supersedes PR #3340.
@SethSmucker SethSmucker force-pushed the task/ima-remove-the-rest branch from f92c5a3 to 03036a7 Compare January 22, 2026 14:02
SethSmucker added a commit that referenced this pull request Apr 14, 2026
Re-apply #3345 changes on top of current #3253 base (0d73343).
Cherry-picked ce539af and resolved 6 conflicts by taking the
current base's test structure (AbstractQueryTest framework) with
InMemoryInstance swapped to InMemoryAccumulo.

BulkInputFormat: only swapped the InMemoryInstance constructor call.
getTabletLocator/TabletLocator code left as-is (will be replaced
when #3449 merges).

Version alignment: root pom in-memory-accumulo version bumped from
4.0.5-SNAPSHOT to 4.0.6-SNAPSHOT to match module pom.

Fixes #2597
Part of #2443
Comment thread warehouse/core/src/main/java/datawave/mr/bulk/BulkInputFormat.java Outdated
ivakegg
ivakegg previously approved these changes Apr 14, 2026
apmoriarty
apmoriarty previously approved these changes Apr 17, 2026
Comment thread warehouse/core/src/main/java/datawave/mr/bulk/BulkInputFormat.java Outdated
Comment thread warehouse/core/src/main/java/datawave/mr/bulk/BulkInputFormat.java Outdated
InMemoryAccumuloClient:
- Remove unused ConditionalWriterConfig field and ensureOpen method
- Remove AccumuloSecurityException from constructor (not thrown)
- Remove unused createConditionalWriter(String) overload
- Remove TableNotFoundException from createConditionalWriter (not thrown)
- Move inline comments to javadoc

InMemoryConnector:
- Remove AccumuloSecurityException from constructor (not thrown)
- Move inline comment to javadoc

BulkInputFormat:
- Simplify online check per ddanielr: replace tableId/tableIdMap/isOnline
  block with single isOnline(tableName) call
- Remove unused tableId variable and TableDeletedException import

Part of #2443
@SethSmucker SethSmucker dismissed stale reviews from ivakegg and apmoriarty via d7d4e81 April 28, 2026 15:03
@SethSmucker SethSmucker requested a review from lbschanno May 5, 2026 20:23
Copy link
Copy Markdown
Collaborator

@foster33 foster33 left a comment

Choose a reason for hiding this comment

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

When using the changes to in-memory-accumulo introduced in this PR, there is a compilation failure:

warehouse/query-core/src/test/java/datawave/query/util/MockMetadataHelper.java:[89,11] exception org.apache.accumulo.core.client.AccumuloSecurityException is never thrown in body of corresponding try statement

Unsure if this is related to this PR or if it is an issue with the current SNAPSHOT version

@SethSmucker
Copy link
Copy Markdown
Collaborator Author

When using the changes to in-memory-accumulo introduced in this PR, there is a compilation failure:

warehouse/query-core/src/test/java/datawave/query/util/MockMetadataHelper.java:[89,11] exception org.apache.accumulo.core.client.AccumuloSecurityException is never thrown in body of corresponding try statement

Unsure if this is related to this PR or if it is an issue with the current SNAPSHOT version

4.0.4 is still being used on integration, so until we update to 4.0.6-SNAPSHOT we won't be able to use any of the changes here even though we build them.

When using the changes to in-memory-accumulo introduced in this PR, there is a compilation failure:

warehouse/query-core/src/test/java/datawave/query/util/MockMetadataHelper.java:[89,11] exception org.apache.accumulo.core.client.AccumuloSecurityException is never thrown in body of corresponding try statement

Unsure if this is related to this PR or if it is an issue with the current SNAPSHOT version

I just swapped over from 4.0.4 to 4.0.6-SNAPSHOT, it should be good to go now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants