Add early validation for transaction mode compatibility in the data loader import command #3186
Conversation
…oader import command
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an early validation step in the data loader's import command to check for transaction mode compatibility. This is achieved by attempting to start a read-only transaction before the import process begins, allowing for early failure with a clear error message if the configuration is incompatible. The implementation includes a new validateTransactionMode method and associated unit tests. Additionally, the creation of TransactionFactory has been refactored to happen only once, improving efficiency and testability. The changes are well-implemented, but there is a minor issue in one of the new tests regarding how an exception is mocked, which I've commented on.
...der/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR adds early validation for transaction mode compatibility in the data-loader import command by attempting to start a read-only transaction during initialization. This helps detect incompatible transaction manager configurations (e.g., single-crud-operation) before data processing begins, providing clear error messages to users.
Key Changes:
- Added
validateTransactionMode()method that attempts to start and abort a transaction when in TRANSACTION mode - Refactored
call()method to createTransactionFactoryonce and reuse it across validation and import operations - Added
INVALID_TRANSACTION_MODEerror enum toDataLoaderError
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| DataLoaderError.java | Added new error enum INVALID_TRANSACTION_MODE for transaction mode validation failures |
| ImportCommand.java | Added validateTransactionMode() method, refactored call() to create and reuse TransactionFactory, updated method signatures to accept transactionFactory parameter |
| ImportCommandTest.java | Added two unit tests covering validation behavior for specific transaction errors and generic exceptions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java
Outdated
Show resolved
Hide resolved
thongdk8
requested review from
a team,
Torch3333,
brfrn169,
feeblefakie,
inv-jishnu,
komamitsu and
ypeckstadt
and removed request for
a team
brfrn169
left a comment
brfrn169
left a comment
There was a problem hiding this comment.
Left several comments. PTAL!
data-loader/core/src/main/java/com/scalar/db/dataloader/core/DataLoaderError.java
Outdated
Show resolved
Hide resolved
data-loader/core/src/main/java/com/scalar/db/dataloader/core/DataLoaderError.java
Outdated
Show resolved
Hide resolved
...-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java
Outdated
Show resolved
Hide resolved
...-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java
Outdated
Show resolved
Hide resolved
...-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java
Show resolved
Hide resolved
…oader import command (#3186) Co-authored-by: Peckstadt Yves <[email protected]>
7 participants
Description
This PR adds early validation for transaction mode compatibility in the data-loader import command. The validation ensures that the configured transaction manager supports transaction mode before attempting to import data.
This also works for
scalardb-cluster-data-loaderwhere currently the client cannot determine the actual transaction manager configured on the cluster side (e.g.,single-crud-operation,consensus-commit). By attempting to start a transaction early, we can detect incompatible configurations and provide clear error messages before processing begins.Related issues and/or PRs
N/A
Changes made
validateTransactionMode()method inImportCommandthat attempts to start a read-only transaction when in TRANSACTION mode to verify compatibilityUnsupportedOperationExceptionspecifically when transaction mode is not supported (e.g., single-crud-operation configuration)TRANSACTION_MODE_VALIDATION_FAILEDfor generic validation failuresChecklist
Additional notes (optional)
NA
Release notes
Added validation for transaction mode compatibility in data-loader import command to detect configuration issues early and provide clear error messages.