Compatibility with Django 4.0

chris-allan commented 1 year ago

Introduction

This PR is a follow on from the Django 3.2.x upgrade #356 and #435. Django 3.2.x leaves LTS in 12 months in April 2024 ^1 so it would be prudent to make ourselves ready for that by getting the codebase prepared to first support at least Django 4.0.x. Django 4.0.x went end of life in August 2022.

The strategy followed in this PR is to first allow Django 4.0.x and then make all the changes in a way that is also still compatible with Django 3.2. This should allow us to work with both versions and then revert the singular commit allowing Django 4.0 if desired and when we are ready.

Release notes for the major versions between our current 3.2.x and the 4.0.x target:

https://docs.djangoproject.com/en/4.2/releases/4.0/

The django-pipeline dependency has been updated to the latest version in order to support Django 4.0.x. pytz support has been deprecated and the dependency removed from Django itself so it has been added. All other dependencies have been left alone.

NOTE: The supported Python versions for Django 4.0.x are currently 3.8, 3.9, and 3.10. 3.11 is only supported from Django 4.1.x forward.

Proposed path to release

Decide on the new target version; I've selected 6.0.0 for the purposes of discussion
Release new versions of several OMERO.web plugins/applications with version pinning of omero-web to <6.0.0
Publicly announce that omero-web 6.0.0 will include support for Django 4.0.x (which results in all deprecations in 3.0.x and 3.1.x being dropped) and publish the ideal timeline
Deploy the test candidate and iterate until we are comfortable enough to publish a release candidate for community consumption
Continue to iterate, publishing new release candidates if required, until we are comfortable enough to make a release; this may include being comfortable with a subset of Django 4.0.x compliant OMERO.web plugins/applications
Make new releases of compliant OMERO.web plugins with version pinning of omero-web to >=6.0.0

Per-version specific concerns

Django 4.0.x (deprecations from Django 3.0.x removed)

See:

https://docs.djangoproject.com/en/4.2/releases/3.0/#deprecated-features-3-0

Detail:

django.utils.translation.ugettext(), ugettext_lazy(), ugettext_noop(), ungettext(), and ungettext_lazy() are deprecated in favor of the functions that they’re aliases for: django.utils.translation.gettext(), gettext_lazy(), gettext_noop(), ngettext(), and ngettext_lazy().
To avoid possible confusion as to effective scope, the private internal utility is_safe_url() is renamed to url_has_allowed_host_and_scheme(). That a URL has an allowed host and scheme doesn’t in general imply that it’s “safe”. It may still be quoted incorrectly, for example. Ensure to also use iri_to_uri() on the path component of untrusted URLs.

Django 4.0.x (deprecations from Django 3.1.x removed)

See:

https://docs.djangoproject.com/en/4.2/releases/3.1/#deprecated-features-3-1

Detail:

The HttpRequest.is_ajax() method is deprecated as it relied on a jQuery-specific way of signifying AJAX calls, while current usage tends to use the JavaScript Fetch API.
django.conf.urls.url() alias of django.urls.re_path() is deprecated.
The {% ifequal %} and {% ifnotequal %} template tags are deprecated in favor of {% if %}.

sbesson commented 1 year ago

Decide on the new target version; I've selected 6.0.0 for the purposes of discussion

I'll read the details but conceptually, no objection to targeting a major version increment of OMERO.web. If that's the collective decision, one thing to bear in mind is that we might want to bump https://github.com/ome/omero-web/blob/b3a74ac8f4081f60e955c750b68fea4f3e747a29/omeroweb/version.py#L10 to 6.0.0.dev0 and start reviewing a few locations in the codebase dealing with the Web/server compatibility like https://github.com/ome/omero-web/blob/6e25b39df40dbe6e84994430a022be369898c3a3/omeroweb/connector.py#L264-L274

Although it is related, this is conceptually different from the core changes here so happy to move this into a separate PR.

chris-allan commented 1 year ago

Yeah, the last time I think we decided 5.12.0 or whatever it was instead of 6.0.0. The changes here could easily be 5.20.0.

snoopycrimecop commented 1 year ago

Conflicting PR. Removed from build OMERO-python-superbuild-push#1424. See the console output for more details. Possible conflicts:

PR #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/templates/webclient/annotations/includes/toolbar.html
- omeroweb/webclient/urls.py
PR #457 sbesson 'Declare portalocker as external dependency'
- setup.py

~~--conflicts~~ Conflict resolved in build OMERO-python-superbuild-push#1430. See the console output for more details.

chris-allan commented 1 year ago

@jburel: Force pushing these commits messes and rewriting all the history here messes up some other workflows I have going. Is the only thing that needed to happen a fix of the merge conflict from master as a result of #457 getting merged?

jburel commented 1 year ago

Sorry for messing the history and the workflows. This is the only conflict

chris-allan commented 1 year ago

Understood. I'll merge master in and force push again unless you're depending on the new HEAD here?

jburel commented 1 year ago

Nothing depends on it at the moment

chris-allan commented 1 year ago

:+1:

All done. Had to re-run one of the jobs due to a PyPI gateway timeout when downloading one of the wheels.

jburel commented 1 year ago

Great.This will be included tomorrow in our CI. Apologies for breaking your workflows

will-moore commented 1 year ago

Testing locally and on merge-ci - all working fine.

will-moore commented 1 year ago

NB: removed my comment about failing tests above as Java tests are also failing so not due to this PR.

will-moore commented 1 year ago

Opened PRs for various web apps (see links above ⬆️)

I know that for the last Django upgrade we released all the apps with omero-web dependency pinned, but then we removed that and re-released them all before the omero-web Django upgrade was released. So this seems like a fair bit of unnecessary extra work.

chris-allan commented 1 year ago

Since all these changes are completely valid when in use with Django 3.2 we don't necessarily have to follow that pattern. However, I'd expect we will have to when we raise the Django requirement to 4.2. It'll be compatibility chaos otherwise.

will-moore commented 1 year ago

Excluding to allow merging of #466 which conflicts...

EDIT: remove flag since it's now merged

will-moore commented 1 year ago

@chris-allan the latest release created merge conflicts with this, if you want to fix now?

Next steps? Lots of web app PRs now available to review (see above)

chris-allan commented 1 year ago

@chris-allan the latest release created merge conflicts with this, if you want to fix now?

:+1: Done.

Next steps?

Given the number of template and template tag changes here I'd say this needs a full testing workup. The other projects are mostly quite straight forward so if you want to start there I'd get those reviewed and merged. There are no changes in this PR that are not compatible with Django 3.2 so these can be included in full or in part whenever we're ready.

will-moore commented 1 year ago

Checking pages with updated templates on merge-ci (with Django 3.2.19):

Edit experimenter form
Activities with various jobs - script, delete, chgrp
Partial delete/chgrp of Fileset
Right panel toolbar
Tags in right panel
Hierarchy panel
Preview panel owner thumb
Tags page
Group / User change menu
Change group target
pagination controls
history page
scripts dialog
search form

will-moore commented 1 year ago

@jburel I tried updating merge-ci to install Django 4.0 but this failed https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-web/1664/console since it's python 3.6 and Django 4.10 only supports python 3.8+ https://docs.djangoproject.com/en/4.2/releases/4.0/

jburel commented 1 year ago

ok So we will need to evaluate it in the separate CI

will-moore commented 1 year ago

As discussed in web meeting today, since merge-ci is currently on python 3.6, Django 4.0 can't be installed. Until that changes, we should probably stay pinned to Django 3.2, since we could otherwise accidentally introduce something incompatible with Django 4.0 and it would go undetected.

sbesson commented 1 year ago

No objection to keeping the mainline compatible with Django 3.2 immediately as this should also keep the master branch releasable at any point.

In order to be able to be continue the testing with Django 4+, can I suggest we only partly revert c9191f7e6f5284016fe70f0c89eaba3efda1474c and only cap in setup.py for now? That should allow testers to manually pip install Django==4.0 in their environments while preventing the plugin to fail at runtime due to the additional version check.

chris-allan commented 1 year ago

LGTM - thanks. "django-pipeline==2.1.0" is compatible with Django 3.2 so it's fine to keep that change.

Just make sure it's still okay with Python 3.6 in the current Dundee CI environment. If it's a problem, let me know and we can revert e0a7d685280e1b463cb0b9faa1b8efdb08dc99f4 as well.

Edit: Same for pytz as well.

will-moore commented 1 year ago

Hmm - yes I see that Python 3.6 was removed from the classifiers in latest release: https://github.com/jazzband/django-pipeline/blob/2.1.0/setup.py

However, merge-ci is running on Python 3.6 and running omero web start which runs the collectstatic which uses pipeline (I think). I don't think that the deployment uses the output of django-pipeline, so I don't think we have any validation of those generated assets. So it's probably OK, but reverting that change is probably less work than checking that django-pipeline 2.1.0 works on python 3.6.

ome / omero-web