Add support for django-mysql's JSONField as alternative implementation

[cb109]

What

This PR adds optional support for django-mysql's JSONField (supported by MySQL 5.7+) to be used instead of the already supported django-jsonfield and Django TextField fallback.

Why

We are using MySQL and django-mysql's JSONField already and would like to use this library without having to rely on a different JSONField implementation. See https://github.com/justquick/django-activity-stream/issues/415

How

The new actstream.jsonfield module is responsible to choose a suitable JSONField implementation on startup:

• USE_JSONFIELD = True and django-jsonfield + django-jsonfield-compat installed: Use that.
• USE_JSONFIELD = True and django-mysql installed: Use that instead.
• USE_JSONFIELD = False: TextField Fallback is used.

A debug print like JSONField implementation is: <class 'django_mysql.models.fields.json.JSONField'> is done on startup to be able to manually check whether the correct implementation is chosen.

Testing

There already is a test named TestAppTests.test_jsonfield() that checks that attaching data to the JSONField works as intented, making that pass is the goal.

I have updated the tox configuration to use django-mysql for the sql environments instead of django-jsonfield. Please see https://tox.readthedocs.io/en/latest/config.html#complex-factor-conditions for syntax.

I have done local test runs with Python 2.7 and 3.7 against Django 1.11 and 2.1 using SQLite and MySQL 5.7:

tox -e py27-django111-mysql
tox -e py27-django111-sqlite
tox -e py27-django21-mysql
tox -e py27-django21-sqlite
tox -e py37-django111-mysql
tox -e py37-django111-sqlite
tox -e py37-django21-mysql
tox -e py37-django21-sqlite

[cb109]

I see that a couple of travis builds are failing, possibly due to library incompatibilies. I'll see if I can fix that.

[cb109]

I was able to fix the builds by telling travis to use mysql 5.7, I hope that is acceptable. Using a mysql version lower than that results in django-mysql throwing an exception giving a helpful explanation, which I think is ok enough to handle that case.

[cb109]

@auvipy Thank your for taking a look at this! Can you please elaborate a bit what you'd want me to do?

it would be great to have generic datafield based on pg,my n sqlite

As far as I understood, django-jsonfield-compat already abstracts the JSONField in a way that people can use both Django/Postgres builtin JSONField as well as the alternative implementation that uses a TextField internally. This PR simply allows to choose an alternative implementation from django-mysql, but does prefer the other implementations that existed before. Is there something missing you would like me to add?

[coveralls]

Coverage increased (+0.07%) to 94.232% when pulling d02d81a3b5850463a030b2b670fb2f087592cd67 on cb109:issue-415-support-django-mysql-jsonfield into d27ecef5c2e5fef361e620a9008592c4a2985786 on justquick:master.

[auvipy]

after having another careful look and reading your replies, your approach seems alright. could you check for documentation update regarding this change?

[auvipy]

and thanks for tackling this.

[cb109]

@auvipy thank you for reviewing :+1: I will update the documentation tomorrow when I am back at work

[cb109]

@auvipy I did update the documentation, but I was unable to build the docs locally to check formatting. I am running into the following problems:

• Calling make docs from the root directory will always tell me that make: 'docs' is up to date., which is not true, as I have made changes.
• Trying to force a docs build by cd docs/ and then calling make html gives me a ModuleNotFoundError: No module named 'actstream.runtests'

Any ideas what I have to do to make it work?

UPDATE: Nevermind, I made it work. Had to change

# conf.py
# os.environ['DJANGO_SETTINGS_MODULE'] = 'actstream.runtests.settings'
os.environ['DJANGO_SETTINGS_MODULE'] = 'runtests.settings'

and add

# runtests/settings.py
import sys
sys.path.insert(0, os.path.dirname(__file__))

and then I was able to run make html. I did of course not commit this, just hacked locally. Can you confirm or correct me on how this is supposed to be done the right way?

[auvipy]

polish it first. I will tackle the swappable model pr. hope you will put your input there.

[cb109]

@auvipy Will watch that PR. From my point this one is already somewhat polished: Do you have some pointers what you would like me to iterate on? Add tests for the imports maybe?

[cb109]

Thank you for reviewing and merging :+1:

[auvipy]

you can come with additional tests and improvements regarding this if you want.