Skip to content

Introducing DynamoDB interception via SDK client class replacement#1495

Open
aschenzle wants to merge 10 commits intoWebFuzzing:masterfrom
aschenzle:pr/dynamodb-interception
Open

Introducing DynamoDB interception via SDK client class replacement#1495
aschenzle wants to merge 10 commits intoWebFuzzing:masterfrom
aschenzle:pr/dynamodb-interception

Conversation

@aschenzle
Copy link
Copy Markdown
Collaborator

Intercepting main operations (Query, Scan, GetItem, BatchGetItems, PutItem, UpdateItem and Delete Item) for both sync and async DynamoDB SDK clients. The interception happens at the SDK level and I've verified it works via an integration test using Docker. There was a dependency conflict while bringing the AWS SDK through Maven, Netty versions would get mixed up for Redis end to end tests so I've made the decision to exclude Netty from Lettuce package and use the Netty version packaged on AWS SDK. All e2e tests are passing.

As this is my first PR I'm looking for exhaustive feedback to quickly learn any conventions I've might missed.

Collections.sort(tableNames);
return tableNames;
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
return Collections.emptyList();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is is defaulted to empty?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed like the best option, as we don't want the interception to throw exceptions and retuning null requires handling it. We are just getting less metadata about the interception for this particular request if for some weird reason it fails (Only case I can think this failing would be a dependency problem with another version of the SDK breaking API compatibility, not expected from AWS).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you encapsulate this and throw a RuntimeException?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just committed a refactored version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants