django-simple-audit icon indicating copy to clipboard operation
django-simple-audit copied to clipboard

[WIP] Add Support for Django 1.11 and add Python 3 support.

Open WTFox opened this issue 7 years ago • 2 comments

Adds support for Python 3 and Django 1.11.

A high level overview

  • Removes the use of django.conf.urls.patterns since support was added for plain lists.
  • Replaces instances of unicode with str. Also adds python_2_unicode_compatible to Django models so that only __str__() has to be implemented.
  • Casts dict.keys() to a list since it's not that.
  • Removes instances of del dict[some_key] in iterations to avoid throwing a Iterable size changed during iteration error.
  • Adds a helper function to tests to make comparing lists of dicts more consistent.
  • Updates the project meta information.

Notes:

  • django.conf.urls.patterns support was removed in 1.8, so I've updated the readme to suggest using Django >= 1.8
  • This project is compatible with python >= 2.7 and python 3 >= 3.6.

Dangers with upgrading to Python 3

While this library works out of the box with Python 3 there is a caveat that could cause unexpected side effects. For that to happen the following conditions have to be true:

  • You have a GenericRelation to simple_audit.Audit from a Model that you've registered with Simple Audit
  • You're upgrading from Python 2 to Python 3

Lets say that a model contains a GenericRelation back to simple_audit.Audit so that the audits can be queried like instance.audits.all() and a new row gets added to the db. Simple Audit will mark that as an ADD operation and will create a record for it. It will look for all fields on the model and get the new and existing values for each one. It works as expected for all fields until it gets to our audits field, the GenericRelation.

What happens is that it looks for the value and calls unicode on it. The value in this case is a GenericRelationObjectManager instance, but calling str() on it will resolve it to simple_audit.Audit.None. This went largely unnoticed because the subsequent changes to the model would produce the same value for audits.

Python 3 comes in like a wrecking ball and obviously unicode() is no longer available. We have str() now for string types. Replacing the unicode usage with str does something entirely unexpected. Instead of resolving to the same simple_audit.Audit.None string, it now produces a string of the objects repr with memory locations e.g. '<django.contrib.contenttypes.fields.GenericRelatedObjectManager object at 0x10a72d510>'

This matters because when Simple Audit gets triggered, it gets the existing value and new instance value of all fields on the model - including fields that have a GenericRelation back to simple_audit.Audit. Which means there will always be a difference because of memory locations being printed in the string.

Long story short, upgrading to Python 3 can and probably will add extraneous information to your audit_change table and will muddy the information making it harder to digest.

WTFox avatar Jan 23 '18 19:01 WTFox

@leandrosouza Do you know when you'll be able to look at this? Not trying to push, just curious. Thanks for your help! :)

WTFox avatar Jan 26 '18 15:01 WTFox

I just noticed that the quoting is a bit off. Marking this as a WIP until I fix it and push.

WTFox avatar Jan 29 '18 17:01 WTFox