Skip to content
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 partition/sort key getters to Item trait #146

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ramosbugs
Copy link

What did you implement:

The Item trait currently includes a key() method that returns a map from attribute name to attribute value for either the partition key only or both the partition and sort keys. In the latter case, it's not possible to distinguish between the partition and sort key attributes without separate knowledge about which key is which.

This PR adds partition_key() and sort_key() methods to the Item trait, which allow each key to be accessed separately. This makes it easier to write middleware and other generic code that works on multiple item types. Both methods are automatically implemented by the derive proc macro.

How did you verify your change:

Added some test cases, plus tested the code manually from my (separate) downstream crate.

What (if anything) would need to be called out in the CHANGELOG for the next release:

Since this PR adds new methods to the Item trait without providing a default implementation, it constitutes a breaking change for users that manually implement this trait rather than using the derive proc macro.

@phrohdoh
Copy link
Contributor

Would you please rebase this on top of current master to address CI issues unrelated to this PR (which have since been fixed)?

@zamazan4ik
Copy link

@ramosbugs I've added your PR in this fork: rust-serverless@bad13f5

If you have any additional ideas/issues/PR - please send them in the fork above since this repo is not actively maintained by the original author.

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.

3 participants