-
Notifications
You must be signed in to change notification settings - Fork 7
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
Marshmallow serialization 🔥️ #75
Conversation
cce62a0
to
8cd0a80
Compare
created_at = fields.DateTime(dump_only=True) | ||
deleted_at = fields.DateTime(dump_only=True) | ||
updated_at = fields.DateTime(dump_only=True) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your assumptions around created_at deleted_at etc are valid and make sense. The note around id, created_at, deleted_at, and updated_at being read-only by default ought to find its way into the docs because someone will lose their mind trying to figure out why it doesn't work to write. I'm not sure if Worf should set these fields to read-only or leave that to the users to implement, but I'm fine starting here.
I imagine all it takes to override this is to set it on the serializer that inherits, so no worries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided not to set these by default but instead add a mechanism for settings these via django settings
, along with any other default serializer meta you might want to apply, saving clients from creating an abstract base serializer, or needing to extend metaclasses to do it, which also means I can leave the implicit id
alone and let it coerce to String or Int.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, +1 for checking the box "this pr includes documentation" b/c it's far easier to keep the docs up-to-date along the way than to wait and then overhaul them
89157ee
to
2a02579
Compare
bca3e5e
to
b53cc7c
Compare
b53cc7c
to
f6947b0
Compare
What's this PR do?
Support marshmallow-based serialization so we can define fields once, in snake case, and without any of the cruft.
Where should the reviewer start?
Here's an example, omit the
read
/write
methods and supply a metaclass withfields
to define both:Marshmallow allows you to mark a field as read-only/write-only with
dump_only
/load_only
kwargs to the field.Anything not present in
Meta.fields
won't be serialized, worf assumes thatid
/created_at
/updated_at
/deleted_at
aredump_only
given they're typically read-only, also fyifields.Function
are assumed to bedump_only
.Why is this important, or what issue does this solve?
Separate read/write definitions is a recipe for inconsistency, and largely a relic of how everything worked pre-serialization, this builds out those same things but from a single definition that's more flexible.
What Worf gif best describes this PR or how it makes you feel?
Tasks
README
updates reflecting any new features/improvements to the framework