Copilot commented on code in PR #79:
URL: https://github.com/apache/pulsar-connectors/pull/79#discussion_r3555194157
##########
dynamodb/src/main/java/org/apache/pulsar/io/dynamodb/DynamoDBSourceConfig.java:
##########
@@ -184,11 +184,12 @@ protected Region regionAsV2Region() {
public AmazonDynamoDBStreams
buildDynamoDBStreamsClient(AwsCredentialProviderPlugin credPlugin) {
AmazonDynamoDBStreamsClientBuilder builder =
AmazonDynamoDBStreamsClientBuilder.standard();
+ // EndpointConfiguration already carries the signing region, and the
AWS SDK v1 forbids
+ // setting both an endpoint configuration and a region, so they must
be mutually exclusive.
if (!this.getAwsEndpoint().isEmpty()) {
builder.setEndpointConfiguration(
new
AwsClientBuilder.EndpointConfiguration(this.getAwsEndpoint(),
this.getAwsRegion()));
Review Comment:
The endpoint/region exclusivity and per-client endpoint gating logic here
fixes a previously user-facing crash (IllegalStateException from AWS SDK v1),
but it isn’t covered by existing unit tests (current tests only validate config
loading/arg checks). Please add a regression test that constructs a config with
endpoint + region set and asserts `buildDynamoDBStreamsClient` /
`buildDynamoDBClient` / `buildCloudwatchClient` do not throw and select the
intended endpoint field.
--
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]
