Add support for Django 1.11 and Python 3

[WTFox]

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]

@sburns This seems ready according to CI, but I'd like to test this out in our app first. Not exactly sure how to accomplish that so looking for your advice.

[sburns]

In general I think this looks fine, but I do want to test in our staging environment. Can you build a playbook of things to exercise to ensure simple audit is still working (ie what models we audit, where to see they're still working, etc)

[sburns]

str on python2 does very different things than unicode. Should we bring in six and use six.text_type?

[WTFox]

I don't think that's necessary in this case. str and unicode definitely behave differently, but with the unicode_literals import at the top of this file it should bridge the gap where needed. Also, this is just storing a string in the db logging what changed and how it changed.

This is what I was using for reference link

[sburns]

unicode_literals stops you from having to type the u on u'my hardcoded string'.

[WTFox]

Ah, good catch. I can bring in six.text_type and replace the instances of str with that.

[sburns]

As far as how to test in staging, we can point our reqs.txt to this version. We may want to reach out to @leandrosouza and see if there's a possibility of us taking over maintainer rights so we could cut a release to PyPI

[leandrosouza]

Don't worry guys I will test by tomorrow and deploy a new version to Pypi

[sburns]

@WTFox can you make a PR to his fork?

[WTFox]

@WTFox can you make a PR to his fork?

Submitted leandrosouza#27

[WTFox]

As stated in the PR body, there will be some differences when upgrading to Python 3. We have two ways of mitigating this as far as I can tell.

1. Implement a solution to specify fields to ignore when auditing a Model.*
2. Remove the GenericRelations to simple_audit.Audit and do reverse lookups with Django's ContentTypes

• optionally ignore fields based on a [] variable in the Model's Meta. If we specify in there what fields we want to ignore auditing, we can exclude those fields in the to_dict() method that builds dictionaries to compare.

[sburns]

I think it will be easier and less intrusive to simple-audit for us to remove our GenericRelations to simple_audit.Audit and write a shareable function to take an instance and return a queryset to its audits. We could then do something like

class RegisteredModel:
    ...

    @property
    def audits(self):
        return function_to_get_audits(self)

And then not need much of an outward API change for ourselves because instance.audits will still do what anything else expects.

[briandailey]

@sburns @WTFox what happened with this? not important anymore?

[WTFox]

@sburns @WTFox what happened with this? not important anymore?

@briandailey We're actually pinned to this commit in our requirements.txt. The author was going to review this and merge it upstream, but it seems that never happened. So, being that this is (and has been) used in production for 4 months 😬 , I'd say this can be merged.