Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wagtail 2.10 request.site is deprecated. #185

Closed
DIGIREN opened this issue Aug 15, 2020 · 4 comments
Closed

Wagtail 2.10 request.site is deprecated. #185

DIGIREN opened this issue Aug 15, 2020 · 4 comments

Comments

@DIGIREN
Copy link

DIGIREN commented Aug 15, 2020

In wagtail 2.10, request.site is deprecated and must be replaced with Site.find_for_request(request). In most cases it still works, but I've encountered a couple random scenarios where the use of request.site in wagtailtrans.middleware.TranslationMiddleware has resulted in WSGI errors. I've fixed this myself in my project by copying translationmiddleware into my own file and swapping the request.site for Site.find_for_request(request), but this will need to be fixed in order to maintain compatibility with 2.10.

As far as I can tell, this is the only incompatibility with 2.10 so far, ive been using wagtailtrans on it for a couple days now and i've been doing some testing and have had no issues except the wsgi errors caused by that incompatibility. The package will also need updated to be compatible with <=wagtail 2.10 as right now it is set at <wagtail 2.10, and starting in October, pip will use a new dependency resolver that does not allow installing a package with incompatible dependencies.

If its acceptable to you guys, I could probably have a PR for this in like a week if I get the time, if someone else wants to hop on it faster though by all means go for it, these changes shouldn't be terribly hard to make as far as i can tell.

@jjanssen
Copy link
Member

Hi @DIGIREN, could you provide some more details on what issues you encountered with WSGI? Like for example which WSGI server and version are you running and what exact errors showed up when running? Did the errors also occur when running ./manage.py runserver or just specifically in a WSGI environment.

@DIGIREN
Copy link
Author

DIGIREN commented Aug 24, 2020

Hey @jjanssen, it has been a little while at this point, so i dont 100% remember how I caused it but i know i was using Gunicorn and it was only occurring specifically within the WSGI environment.
I should have time in a few days to boot back into my environment, unpatch it, and re-create the error. Then I can update you with more specific details :)

@jjanssen
Copy link
Member

In addition #188 got merged and published so wagtailtrans is 'officially' compatible with Wagtail 2.10 which solves #186
Although I wasn't able to reproduce the above scenario during the update of the testsuite or briefly running gunicorn against our sandbox.

The deprecation of the SiteMiddleware itself shouldn't be official until the next Wagtail release (2.11). So I'm a bit confused why it is already causing troubles at this stage. I'm keeping this open in the meantime if anyone is able to share more details.

I've also opened a work-in-progress PR (#191) to keep track of the forthcoming 2.11 release. The changes in the Translation middleware are aimed to be backwards compatible for Wagtail 2.7 (LTS). But can be copied to provide an alternate solution for whoever is encountering the same problem until we are able to solve it.

@jjanssen
Copy link
Member

Small update on this. We've released 2.2.1 this week and instead of waiting along on the official deprecation, we've taken into consideration that some are already leaving the SiteMiddleware prior to 2.11 due to the deprecation warning. So with 2.2.1 locally added middlewares can be removed and added again through Wagtailtrans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants