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

Upgrade to django-pghistory 3.4.0 #4441

Merged
merged 11 commits into from
Sep 30, 2024
Merged

Upgrade to django-pghistory 3.4.0 #4441

merged 11 commits into from
Sep 30, 2024

Conversation

quevon24
Copy link
Member

@quevon24 quevon24 commented Sep 10, 2024

  • Update django-pghistory version in pyproject.toml and poetry.lock
  • Update all @pghistory.track decorators in models to use the features of new library version
  • Removed custom django-pghistory tracker, it is deprecated.
  • Migrations for production (migrations only contain triggers)
  • Update tests

Update all @pghistory.track decorators
Migrations for production
@quevon24 quevon24 linked an issue Sep 10, 2024 that may be closed by this pull request
@quevon24 quevon24 marked this pull request as ready for review September 11, 2024 00:12
@mlissner
Copy link
Member

Did you see the setting that's now possible to set a default tracker?

@quevon24
Copy link
Member Author

quevon24 commented Sep 11, 2024

Did you see the setting that's now possible to set a default tracker?

Yes, i was thinking that maybe we could use the PGHISTORY_DEFAULT_TRACKERS for the models like Docket, OpinionCluster, etc and leave the custom decorators for m2m relationships like DocketTags, DocketPanel, etc.

Also should that setting go in the third_party directory or since it is so small should it go in django.py file?

It should look like this:

PGHISTORY_DEFAULT_TRACKERS = (
    pghistory.UpdateEvent(
        condition=pghistory.AnyChange(exclude_auto=True), row=pghistory.Old
    ),
    pghistory.DeleteEvent(),
)

@albertisfu
Copy link
Contributor

@mlissner I reviewed this in detail, and it works properly after the update.

Just a couple of comments:

  • We don't need SQL migrations for the trigger changes since we don't have triggers on replicas, right?
  • Previously, m2m through tables generated events on updates and deletions. With this update, m2m tables now generate events only on the first insert and on delete. This makes sense because m2m instances can only be added or deleted (there are no updates).
  • One change to note when implementing an admin for event tables is that we'll now have two types of pgh_labels:
    • The old one: update_or_delete_snapshot, which doesn't distinguish between updates and deletions.
    • The new ones: insert (for m2m tables), update, or delete.

Screenshot 2024-09-30 at 3 33 07 p m

@mlissner
Copy link
Member

Sounds good, yes, no triggers on replicas. Does that mean we need tweaks here?

From the db guide:

image

@albertisfu
Copy link
Contributor

Does that mean we need tweaks here?

No, this is good to go! We don't need SQL files since we don't need to apply those trigger changes to either our replica or customer replicas. There are some AlterField migrations, but they are no-op changes for SQL, as they only rename the related_name to "events" for Django.

@mlissner
Copy link
Member

Wonderful. I'll merge and, @quevon24, if you can create a deployment issue for Ramiro, that'd be great.

@mlissner mlissner merged commit 8d1e916 into main Sep 30, 2024
13 checks passed
@mlissner mlissner deleted the 4431-upgrade-to-pghistory-30 branch September 30, 2024 22:42
@mlissner
Copy link
Member

Merged. Thanks everybody!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Upgrade to pghistory 3.0
3 participants