Django version stuck on 2.0 because of Django-suit

amakarudze commented 3 years ago

For a long time, the version of Django used in our website has been stuck at version 2.0 because no work has been done on Django-suit since 2019. We need to be able to move to a newer and supported version of Django while ensuring that there is still a good user interface. The following tasks need to be done:

[x] Update website to use Django admin.
[x] Upgrade Django version to a new and supported LTS.
[ ] Customise the Admin Panel to make it more user-friendly
[x] Upgrade packages/dependencies incrementally.
[x] Upgrade Python version

marksweb commented 3 years ago

Is this best done on a separate branch that can be used by multiple people as it feels like it's going to take quite some time to do?

amakarudze commented 3 years ago

Hadn't thought about that @marksweb but I think that would be a great idea.

amakarudze commented 3 years ago

Is this best done on a separate branch that can be used by multiple people as it feels like it's going to take quite some time to do?

Yes @marksweb, let's use the dev branch for this. We could make this our priority for now.

marksweb commented 3 years ago

Ok sounds sensible @amakarudze

I might try to wrap up where I got to with translations & I can take a look at this. I don't think I was far from complete. I'd certainly been through the apps and some of the templates. Can't be much left for an initial implementation.

marksweb commented 3 years ago

After bumping to django 2.2 for #661 I have also bumped to 3.2 to see how that goes.

Plenty of changes to make here, and upgrades to third party packages.

Strangely though, there seems to be a lot of errors from tests in the SQL syntax. This is unexpected.

They seem to all becoming from a small number of tables, auth_group_permissions, core_user_groups, core_event_team

Some examples;

test setup failed
self = <django.db.backends.utils.CursorWrapper object at 0x7f89f86a3cf8>
sql = 'INSERT INTO "auth_group_permissions" ("group_id", "permission_id") VALUES (%s, %s), (%s, %s) ON CONFLICT DO NOTHING'
params = (56, 45, 56, 46)
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f89f614bc50>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f89f86a3cf8>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.SyntaxError: syntax error at or near "ON"
E               LINE 1: ...p_id", "permission_id") VALUES (56, 45), (56, 46) ON CONFLIC...
E                                                                            ^

self = <django.db.backends.utils.CursorWrapper object at 0x7f89f86361d0>
sql = 'INSERT INTO "core_event_team" ("event_id", "user_id") VALUES (%s, %s) ON CONFLICT DO NOTHING'
params = (10, 34)
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f89f614bc50>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f89f86361d0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.SyntaxError: syntax error at or near "ON"
E               LINE 1: ...ent_team" ("event_id", "user_id") VALUES (10, 34) ON CONFLIC...
E                                                                            ^

self = <django.db.backends.utils.CursorWrapper object at 0x7f89f869f1d0>
sql = 'INSERT INTO "core_user_groups" ("user_id", "group_id") VALUES (%s, %s) ON CONFLICT DO NOTHING'
params = (17, 47)
ignored_wrapper_args = (False, {'connection': <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7f89f614bc50>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f89f869f1d0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.SyntaxError: syntax error at or near "ON"
E               LINE 1: ...r_groups" ("user_id", "group_id") VALUES (17, 47) ON CONFLIC...
E                                                                            ^

I'll have to have a look into what might be special about these. Anybody know of anything significant?

carltongibson commented 3 years ago

That'll be the Postgres version I'd guess. Version 9.6 or higher is required.

marksweb commented 3 years ago

@carltongibson Yes, good point!!

I've still got it pointed at my old 9.4 server because the port wasn't customisable when I started with this project. Moving it to my v12 server works - at least after adding a package to load the .env file into pytest to pickup the different database port.

Down to 7 failed tests now and 2 always fail locally - patreon and lat/long differ on my machine for whatever reason.

marksweb commented 3 years ago

@amakarudze So moving the project to django 3.2 needs postgres 9.6 or higher. However I think 9.6 hits EOL in the coming months.

So it'd be good to understand what version the site currently runs on and what options there may be to upgrade. Version 12 is a good place to be.

marksweb commented 3 years ago

Alright, i've got django 3.2 running and tests passing 👀 🎉

So I think the next steps here are;

Ensure deploy script is using pip-tools to run pip-sync which will keep required packages installed and remove old ones
Merge and deploy #661
Ensure production is using Postgres 9.6 or higher to support django 3
Test & merge django 3.2 PR (yet to be raised)

amakarudze commented 3 years ago

@amakarudze So moving the project to django 3.2 needs postgres 9.6 or higher. However I think 9.6 hits EOL in the coming months.

So it'd be good to understand what version the site currently runs on and what options there may be to upgrade. Version 12 is a good place to be.

@marksweb The server is running Postgres 9.4. I am running Postgres 12 in my own PythonAnywhere personal account. Will need to check PythonAnywhere documentation on upgrading Postgres version. We might need to get help from PythonAnywhere team on this.

amakarudze commented 3 years ago

pip-sync

Noted @marksweb, I will add the command to the deploy script.

amakarudze commented 3 years ago

@amakarudze So moving the project to django 3.2 needs postgres 9.6 or higher. However I think 9.6 hits EOL in the coming months.

So it'd be good to understand what version the site currently runs on and what options there may be to upgrade. Version 12 is a good place to be.

@marksweb We would need to email PythonAnywhere to upgrade our Postgres server to version 12 with times we want this so when we are ready to migrate to Django 3.2 we can ask them to do that for us.

marksweb commented 3 years ago

@amakarudze Thanks for that. Postgres 12 should work on django 2.0, so the current site. So perhaps get in touch with pythonanywhere once the 2.2 release goes out.

I just tagged you in a comment about what the deploy script should ideally do, I think that was on the 2.2 PR.

I actually got the django 3.2 upgrade finished last night. I'm just using the time now to increase the test coverage where I can. I looked at checking a POST to the application apply view because that's not tested, but the way it generates questions makes it a bit difficult to match question_<number> to what data it expects.

And I think I found another way to get the git commit hash for the sentry version linking - python anywhere forums weren't helpful. I'll put a PR together for that once all the django upgrade and translations are complete.

amakarudze commented 3 years ago

@amakarudze Thanks for that. Postgres 12 should work on django 2.0, so the current site. So perhaps get in touch with pythonanywhere once the 2.2 release goes out.

I just tagged you in a comment about what the deploy script should ideally do, I think that was on the 2.2 PR.

I actually got the django 3.2 upgrade finished last night. I'm just using the time now to increase the test coverage where I can. I looked at checking a POST to the application apply view because that's not tested, but the way it generates questions makes it a bit difficult to match question_<number> to what data it expects.

And I think I found another way to get the git commit hash for the sentry version linking - python anywhere forums weren't helpful. I'll put a PR together for that once all the django upgrade and translations are complete.

@marksweb I saw the tag and I replied there, in 2 places actually, thanks.

Great to hear that you finish the Django upgrade to 3.2. Thanks for all your work in all this @marksweb!

I have written to PythonAnywhere so we should do the upgrade of the Postgres server this Friday hopefully.

Great to hear that the Sentry issue is being resolved. Need to know when and why my deploys fail hahaha. While you are working on test coverage, could you kindly look at this issue for providing test coverage to the event application for organizers in organize/views.py? Thanks

amakarudze commented 3 years ago

We are now running Postgres 12 in production so we should be able to upgrade to Django 3.2.

marksweb commented 3 years ago

@amakarudze thats great. I also rebased the 2.2 changes onto the translation PR

amakarudze commented 3 years ago

Finally merged in Django 3.2 updates and upgraded Python version to 3.9 in production. Closing this issue now and the Admin UI can be handled in a separate issue. Thanks @marksweb for all your work in this.

DjangoGirls / djangogirls

Django version stuck on 2.0 because of Django-suit #628