Latest V4.0.1 not working for Mysql Database

[pushyamig]

Describe the bug
I tried to upgrade to new version pinax-eventlog for my app with djangoV3.0.8, Python 3.8,Mysql 5.7.22 and when trying to launch the instance i see this following error. When I looked at the migration file
eventlog.0004_auto_20191205_2033 that is failing on it trying to grab a postgres field/variable (field=django.contrib.postgres.fields.jsonb.JSONField()) which my app doesn't have
I see this issue with both 3.0.1 and 4.0.1 version and currently i was on v2 which did not had a problem. is there a work around for this?

Applying eventlog.0001_initial... OK
Applying eventlog.0002_auto_20150113_1450... OK
Applying eventlog.0003_auto_20160111_0208... OK
Applying eventlog.0004_auto_20191205_2033...Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
   return self.cursor.execute(sql, params)
File "/usr/local/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 74, in execute
    return self.cursor.execute(query, args)
   File "/usr/local/lib/python3.8/site-packages/MySQLdb/cursors.py", line 209, in execute
    res = self._query(query)
   File "/usr/local/lib/python3.8/site-packages/MySQLdb/cursors.py", line 315, in _query
     db.query(q)
  File "/usr/local/lib/python3.8/site-packages/MySQLdb/connections.py", line 239, in query
    _mysql.connection.query(self, query)
 MySQLdb._exceptions.ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'jsonb NOT NULL' at line 1")

Reproduce behavior

1. Upgrade to new version 4.0.1 build and start the instance and when applying migrations the error occurs

Expected behavior
All migrations should apply correctly

Desktop

• Mac

[patrickscottbest]

Hi, the problem lies with the INITIAL migrations that originally shipped to support postgresql only.

The workaround is to blast away all previous migrations for the pinax-eventlog if this is a fresh install.

rm pinax-eventlog/pinax/eventlog/migrations/000*

Then perform the ./manage.py makemigrations and ./manage.py migrate again.

[patrickscottbest]

There's other remnants that need attention as well. For example, the PyPi version lists a requirement of psycopg2-binary , which would be superfluous to a Mysql user.

As for migrations, i'm narrowing down the exact migration file that bonks, but here's what has shipped to date. We may have a problem here where we need to ship either without an initial migration pack, or have a completely different "MYSQL" version of the pinax-eventlog:

0001_initial.py
0002_auto_20150113_1450.py
0003_auto_20160111_0208.py
0004_auto_20191205_2033.py
0005_auto_20200428_2208.py

[patrickscottbest]

looks like 0005 and 0004 are specifically pinned and designed for postgresql...

With my workaround of deleting the pre-packaged migrations, pinax-eventlog works for mysql. And by doing this, the pinax-project kit can work out of the box with mysql as well.

Essentially, it's a pivotal point in time for Pinax i guess. Does Pinax drop support (even if it was there) for MySQL or not? I'm not keen enough to see the ramifications of re-writing previous migration files.

Those answers may be known to @KatherineMichel or @paltman ...

Also of note, i see the latest migration was handled by Django 2.2.12, and the previous was handled by Django 3.0

0005:

# Generated by Django 2.2.12 on 2020-04-28 22:08

import django.contrib.postgres.fields.jsonb
import django.core.serializers.json
from django.db import migrations


class Migration(migrations.Migration):

    dependencies = [
        ('eventlog', '0004_auto_20191205_2033'),
    ]

    operations = [
        migrations.AlterField(
            model_name='log',
            name='extra',
            field=django.contrib.postgres.fields.jsonb.JSONField(encoder=django.core.serializers.json.DjangoJSONEncoder),
        ),
    ]

0004:

# Generated by Django 3.0 on 2019-12-05 20:33

import django.contrib.postgres.fields.jsonb
from django.db import migrations


class Migration(migrations.Migration):

    dependencies = [
        ('eventlog', '0003_auto_20160111_0208'),
    ]

    operations = [
        migrations.AlterField(
            model_name='log',
            name='extra',
            field=django.contrib.postgres.fields.jsonb.JSONField(),
        ),
    ]

[blag]

The workaround is to blast away all previous migrations for the pinax-eventlog if this is a fresh install.

This is 100% horrible advice. Stop telling anybody to do this. This will only fix the immediate problem and cause a lot more down the road when users have to reconcile their migration history with the migration history from this upstream package.

[patrickscottbest]

I can understand where you're coming from, but sometimes we have to bend things to make it fit. I'm in love with Pinax for the site requirement, but my constraint was MySQL galera.

My workaround is certainly not for a novice, and perhaps the project should roll back my change and state SQL support explicitly.

If someone feels comfortable enough tearing apart pre-packaged migrations for a fresh install, they're likely comfortable issuing a version freeze from pip into requirements.txt and baking in the workaround steps into every instance that gets spawned up. Sadly, i've had to put it into K8s entrypoint scripts on custom baked images...

[patrickscottbest]

@pushyamig which solution are you going to go with? Work through the migration issue, or roll back to v2 ?

[pushyamig]

The workaround is to blast away all previous migrations for the pinax-eventlog if this is a fresh install.

are you suggesting to remove below migration when the my app is starting up have a script baked into my app that does this every time?

0004_auto_20191205_2033.py
0005_auto_20200428_2208.py

What if you have a new migration file in future that depend on 0005_auto_20200428_2208.py how should that be handled? do I need to update the migration dependency in my app when starting up?

I don't exactly know what you mean by problem is with initial migration, I don't have problem updating first 3 migration from the v4.0.1 or in v2

0001_initial.py
0002_auto_20150113_1450.py
0003_auto_20160111_0208.py

[pushyamig]

The workaround is to blast away all previous migrations for the pinax-eventlog if this is a fresh install.
rm pinax-eventlog/pinax/eventlog/migrations/000*

If this suggest that delete all the migration and that will leave the table that was created from before in dev/prod etc. But this approach will fail for local development when we use docker container to creating local mysql instance and we won't have eventlog tables.

[blag]

I can understand where you're coming from, but sometimes we have to bend things to make it fit.

You should not bend things past the point at which they break, and your workaround/advice here absolutely will break things. Maybe not today, but they will once users try to upgrade again. @pushyamig is asking exactly the questions they should be.

Proper support for MySQL was never actually working for this project in the first place. PR #31 only updated the models, but did not update the migrations like it should have. That means that it should still work 100% fine with PostgreSQL, but migrations will prevent users from even installing this successfully when using MySQL.

It is not appropriate to remove migrations once they are published. Period. Assume that each individual migration is instantly applied by users as they are merged into master.

I believe the best way to support PostgreSQL and MySQL users at the same time is to tweak the existing migrations files, add a simple database router to this app, and utilize Django's migration hints to selectively run individual migration operations depending on the database engine. Note that this should not be done lightly, and we would want to manually test this against a PostgreSQL instance and a MySQL (variant) instance to ensure that both migration paths are valid.

Since this project was database-agnostic until migration 0004, I think it should try to support multiple databases, within reason.

[patrickscottbest]

So in otherwords, rewrite 0004 and 0005 to check for database type, and apply different migrations from that point forward.

Would you rewrite them as a "skip me" and then introduce a new "for MySQL only" 0006 and hopefully continue down a converged path starting at 0007?

[pushyamig]

I believe the best way to support PostgreSQL and MySQL users at the same time is to tweak the existing migrations files, add a simple database router to this app, and utilize Django's migration hints to selectively run individual migration operations depending on the database engine.

This make sense to me, manually tweaking the migration file based on the DB backend check. It will be good if the pinax team handle it

If I have to delete migration and have additional start up script to handle it will be mess and won't be clean at all.

[blag]

So in otherwords, rewrite 0004 and 0005 to check for database type, and apply different migrations from that point forward.

Yes.

Would you rewrite them as a "skip me" and then introduce a new "for MySQL only" 0006 and hopefully continue down a converged path starting at 0007?

If we're going to have a converged path, then I would not rewrite the migration files as "skip mes". I would try to modify the existing 0004 and 0005 migrations files to perform roughly the same tasks for each database. So if 0004 converts the field to a JSONB field when run against PostgreSQL, then 0004 should also convert the field to MySQL's equivalent of a JSONB field (which I think is just a JSON field).

The actual operations in the operations array of each migration file would have one operation for PostgreSQL and another operation for MySQL, and the database router would decide which one of the two to actually perform.

Edit: Grammar fix.

[KatherineMichel]

I definitely don't want to seriously break anything in this package. I didn't realize there would be a problem with migrations when I merged the PR. It would be good to have a consensus about the best way forward.

[patrickscottbest]

@blag has the way forward.
It will require some hours to develop modifications to migration packs 0004 and 0005. And then to test against the two different databases.

I am not that seasoned a hand and I don't really understand if the concept of "database router" is a simple check of which database is in use from settings.py , or if there's a deeper django meaning of this term I don't yet know.

I'd certainly buy them a coffee if they would whip up a modification quicker than I can get to it...

[paltman]

You can upgrade to v5 if you can take your Django up to 3.1 (v5 requires 3.1).

This version also resets migrations to just an 0001_initial. Since there have been no schema changes, this should be a perfectly fine upgrade for you to make in your site. The only exception would be if you had any migrations that depended on an event log migration that is no longer there, you'd need to edit that to now depend on the 0001_initial.

This new release gets away from database specific json field implementation and leverages Django 3.1's models.JSONField that handles all the different database implementations under the covers for us.

[paltman]

If upgrading to 3.1 is unacceptable we'll need create a 4.x branch and maintain two versions for a bit. But there is another issue you'll hit later on once the migrations are resolved as commented on here:

3e7e0bf#commitcomment-41491081

[jacobwegner]

I've opened #34 which when merged will also enable support for Django 2.2x and 3.0.x

[jacobwegner]

I've just cut a release for v5.1.0 that makes pinax-eventlog compatible with all the database backends supported by Django.

We've also written up instructions on how to install and configure it for use with Django < 3.1