Django upgrades, UUID handling - #340
Conversation
* Upgrade various imports/calls to use Django 4.2+ (used django-upgrade lib) * Handle UUID in changed_fields, was giving issues * Handle async transactions properly
| if isinstance(pks, Iterable): | ||
| pks = (format_primary_key(pk) for pk in pks) | ||
| else: | ||
| pks = format_primary_key(pks) |
There was a problem hiding this comment.
if not an iterable, then what exactly is happening here? it's also been a long time since i've read the flow of how pk_set can be passed in.
There was a problem hiding this comment.
Good call, I'll simplify this and make sure we're testing it in one project too.
Pushed changes.
There was a problem hiding this comment.
@pcraciunoiu have you been able to test out this change in one of your projects?
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm in no rush but I also haven't used this project in a company since 2020. If you're okay to wait on getting some runtime signals, that's stellar! If you're otherwise feeling confident, then we can merge and make a new release.
There was a problem hiding this comment.
Been using it for a few days. The main use case for the app is an audit trail of changes. No issues with these changes that I could see.
I do have a couple more features that I'd like to PR. The main one is to add a metadata with a callback so that using this project allows us to add custom data on top of the change events.
I'll PR those changes separately soon.
There was a problem hiding this comment.
@jheld meaning, I feel good about merging this if you are ready
jheld
left a comment
jheld
left a comment
There was a problem hiding this comment.
i'm probably going to approve this anyway but hoping to get more information about the changes in crud_flows before doing it.
2 participants
Upstreaming more changes from our fork that we've been using in production.
Thanks for reviewing!