nayaverdier / dyntastic

A DynamoDB library on top of Pydantic and boto3.
MIT License
55 stars 12 forks source link

Allow batch_get for models with alias keys #20

Closed strutt closed 3 months ago

strutt commented 5 months ago

Hi!

Love the library. I'm making use of aliases for my hash and range key fields. Currently Dyntastic.batch_get doesn't work in this case. This patches that issue.

Cheers.

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (49b4a33) to head (d7cace3).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #20 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 6 6 Lines 669 669 Branches 127 127 ========================================= Hits 669 669 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nayaverdier commented 5 months ago

I noticed two more issues with the batch_get type checking:

  1. The type hints for the keys is all str. The easy solution is to replace those with Any. The other solution I can think of is (optionally) make Dyntastic an generic that could be used like class SomeTable(Dyntastic[str, int]) to indicate the hash/range key types at the type hint level. (Bonus is we'd be able to propagate those hints to the other functions that accept keys, as well)
  2. When there's a range key, we don't actually validate either key type

Not blocking this PR but a related issue. Let me know if you have any thoughts based on your usage.

strutt commented 5 months ago

The other solution I can think of is (optionally) make Dyntastic an generic that could be used like class SomeTable(Dyntastic[str, int]) to indicate the hash/range key types at the type hint level. (Bonus is we'd be able to propagate those hints to the other functions that accept keys, as well)

I really like this idea, very much in the spirit of pydantic, though obviously more work than the alternative.

nayaverdier commented 5 months ago

I really like this idea, very much in the spirit of pydantic, though obviously more work than the alternative.

Looked more into doing this and it seems like the only way to get that functionality is by having two classes, something like DyntasticHash(Generic[HashKey]) and DyntasticRange(Generic[HashKey, RangeKey]). It might be possible to do it better in the future with changes to typing, but doesn't seem like it is currently.

For now will make the simpler fixes.

nayaverdier commented 3 months ago

@strutt Finally got around to finishing this up, just released as 0.15.0. Let me know if all is working well for you now.