yihua commented on code in PR #8868:
URL: https://github.com/apache/hudi/pull/8868#discussion_r1224755814
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -185,14 +183,14 @@ private void createLockTableInDynamoDB(AmazonDynamoDB
dynamoDB, String tableName
createTableRequest.setBillingMode(billingMode);
if (billingMode.equals(BillingMode.PROVISIONED.name())) {
createTableRequest.setProvisionedThroughput(new ProvisionedThroughput()
-
.withReadCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key())))
-
.withWriteCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key()))));
+
.withReadCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY)))
+
.withWriteCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY))));
}
dynamoDB.createTable(createTableRequest);
LOG.info("Creating dynamoDB table " + tableName + ", waiting for table to
be active");
try {
- TableUtils.waitUntilActive(dynamoDB, tableName,
Integer.parseInt(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT.key())),
20 * 1000);
+ TableUtils.waitUntilActive(dynamoDB, tableName,
Integer.parseInt(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT)),
20 * 1000);
Review Comment:
nit: use `dynamoDBLockConfiguration.getInt`?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -71,19 +69,19 @@ public class DynamoDBBasedLockProvider implements
LockProvider<LockItem> {
private final AmazonDynamoDBLockClient client;
private final String tableName;
private final String dynamoDBPartitionKey;
- protected LockConfiguration lockConfiguration;
+ protected DynamoDbBasedLockConfig dynamoDBLockConfiguration;
Review Comment:
nit: this should be final.
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -201,14 +199,10 @@ private void createLockTableInDynamoDB(AmazonDynamoDB
dynamoDB, String tableName
LOG.info("Created dynamoDB table " + tableName);
}
- private void checkRequiredProps(final LockConfiguration config) {
-
ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_NAME.key())
!= null);
-
ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_REGION.key())
!= null);
-
ValidationUtils.checkArgument(config.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_PARTITION_KEY.key())
!= null);
-
config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_BILLING_MODE.key(),
BillingMode.PAY_PER_REQUEST.name());
-
config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key(),
"20");
-
config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key(),
"10");
-
config.getConfig().putIfAbsent(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_CREATION_TIMEOUT.key(),
"600000");
+ private void checkRequiredProps(final DynamoDbBasedLockConfig config) {
+
ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_TABLE_NAME.key()));
+
ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_REGION.key()));
+
ValidationUtils.checkArgument(config.contains(DynamoDbBasedLockConfig.DYNAMODB_LOCK_PARTITION_KEY.key()));
Review Comment:
nit: move these to the builder of `DynamoDbBasedLockConfig`?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/DynamoDBBasedLockProvider.java:
##########
@@ -185,14 +183,14 @@ private void createLockTableInDynamoDB(AmazonDynamoDB
dynamoDB, String tableName
createTableRequest.setBillingMode(billingMode);
if (billingMode.equals(BillingMode.PROVISIONED.name())) {
createTableRequest.setProvisionedThroughput(new ProvisionedThroughput()
-
.withReadCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY.key())))
-
.withWriteCapacityUnits(Long.parseLong(lockConfiguration.getConfig().getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY.key()))));
+
.withReadCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_READ_CAPACITY)))
+
.withWriteCapacityUnits(Long.parseLong(dynamoDBLockConfiguration.getString(DynamoDbBasedLockConfig.DYNAMODB_LOCK_WRITE_CAPACITY))));
Review Comment:
nit: use `dynamoDBLockConfiguration.getLong`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
