-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add ScalarDB Dao and related files #2417
Conversation
data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDBDao.java
Outdated
Show resolved
Hide resolved
data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDBDao.java
Outdated
Show resolved
Hide resolved
...-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDBManager.java
Outdated
Show resolved
Hide resolved
@komamitsu san, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment. Other than that, LGTM! Thank you!
@@ -0,0 +1,425 @@ | |||
package com.scalar.db.dataloader.core.dataimport.dao; | |||
|
|||
import com.scalar.db.api.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use the wildcard *
for import
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated to removed usage of wildcard.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review, I left some comments, PTAL.
import java.io.IOException; | ||
import javax.annotation.Nullable; | ||
|
||
public class ScalarDbStorageManger { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class ScalarDbStorageManger { | |
public class ScalarDbStorageManager { |
import com.scalar.db.service.TransactionFactory; | ||
import java.io.IOException; | ||
|
||
public class ScalarDbTransactionManger { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class ScalarDbTransactionManger { | |
public class ScalarDbTransactionManager { |
storageAdmin = storageFactory.getStorageAdmin(); | ||
} | ||
|
||
/** @return storage for ScalarDB connection that is running in storage mode */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it repetitive but without doing this, a warning appears when compiling the code. Even if you follow the Google style guide recommendation, another warning is triggered.
warning: [MissingSummary] A summary fragment is required; consider using the value of the @return block as a summary fragment instead.
* @return Distributed Transaction manager for ScalarDB connection that is running in transaction
^
(see https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment)
Did you mean '*Returns distributed Transaction manager for ScalarDB connection that is running in transaction'?
/** @return storage for ScalarDB connection that is running in storage mode */ | |
Returns storage for ScalarDB connection that is running in storage mode | |
/** @return storage for ScalarDB connection that is running in storage mode */ |
...r/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDbStorageManger.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDbTransactionManger.java
Outdated
Show resolved
Hide resolved
data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDBDao.java
Outdated
Show resolved
Hide resolved
data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDBDao.java
Outdated
Show resolved
Hide resolved
@Torch3333 san, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Co-authored-by: Peckstadt Yves <[email protected]>
Co-authored-by: Peckstadt Yves <[email protected]>
Description
I have added scalarDB dao and related files used in data loader
Related issues and/or PRs
Please review this PR once the following PR is reviewed and merged (Once the PR is merged, I will merge the master to this one to resolve the spotbugs issue).
Changes made
I have added scalarDB dao and related files used in data loader
Checklist
Additional notes (optional)
Road map to merge remaining data loader core files. Current status
General
Export
Import
Release notes
NA