Add option for table prefix to DynamoDB #7598
Conversation
|
|
| 42, | ||
| ByteString.copyFrom(new byte[400 * 1024])))) | ||
| .isInstanceOf(ObjTooLargeException.class); | ||
| soft.assertThatThrownBy( |
There was a problem hiding this comment.
Potentially, although I did not modify the body of that test at all so the duplication was there already.
There was a problem hiding this comment.
OK let's leave it, as it's not related to your changes 👍
There was a problem hiding this comment.
Thanks a lot for your contrubution @maxfirman ! Just a couple of minor nitpicks... I hope you do not mind.
| @@ -40,9 +40,21 @@ public String getName() { | |||
| return DynamoDBBackendFactory.NAME; | |||
| } | |||
|
|
|||
| @SuppressWarnings("ClassEscapesDefinedScope") | |||
There was a problem hiding this comment.
IntelliJ flags this annotation as redundant... Could you double check, please?
There was a problem hiding this comment.
I've removed it. Although interestingly my IntilliJ complains when I remove the annotation.
There was a problem hiding this comment.
What does it say? My EAP 2 build is happy :)
There was a problem hiding this comment.
I guess it might be complaining about returning the package-private DynamoDBBackend type, but the interface defines the return value as Backend... I think the code is correct and IJ was overzealous :)
There was a problem hiding this comment.
Correction: I was running my old IJ built for this review before and it did not give that warning. I switched to EAP 3 (just now) and it does indeed issue the warning. Having considered the code carefully, I think the warning is justified.
I also see some usages that rely on the more specific return type... so cleaning this up is probably beyond the scope of this PR. Let's not suppress the warning, though, as a reminder to revisit this.
There was a problem hiding this comment.
We have the same issue in BigTable impl... we return Backend there, not BigTableBackend.
...age/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java
Outdated
Show resolved
Hide resolved
...age/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBBackend.java
Outdated
Show resolved
Hide resolved
Closes #7595