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

SITE_ID dependancy. #96

Open
iraabbott opened this issue Dec 12, 2018 · 4 comments
Open

SITE_ID dependancy. #96

iraabbott opened this issue Dec 12, 2018 · 4 comments

Comments

@iraabbott
Copy link

Hi,

First, thanks for putting this out there - I plan on using it or some variant, but am running in to issues with multiple sites. There are two 'modes' of use of django.contrib.sites - the original (SITE_ID = ) and 'by request' (no SITE_ID). Some packages make use of the second method, things like allauth for social authentication, for instance. I intend to run my site this way and like the way dbtemapltes is set up, but cannot run because of the dependancy to SITE_ID.

Unfortunately, loaders don't have access to the request for good reason, they may be used elsewhere.

One way I can solve this is with a 5-6 line patch to django itself. I have proposed this to the django development group with muted response, but without outright "won't fix" ("maybe, someday"). No comments yet.

Alternately, I may modify this thing to take a setting if present, otherwise call get_current. I believe this would create a version of dbtemplates which works identically for all installations currently using SITE_ID and allow friendly operation with "by_request". As far as I can see, the only catch is making sure settings.DBTEMPALTES_SITE_ID is in the M2M for all templates.

This scheme would effectively combine the template namespace to the 'main', but I'll probably just string mash the site pk in to the template name under the hood.

Thoughts? Should I go to the effort to try to generalize and prepare a pull request?

Regards,

Ira

@bahoo
Copy link

bahoo commented Sep 13, 2020

This is an old issue but I'm running into something similar, and found @iraabbott's two threads on the Django developer's group, which hopefully Ira doesn't mind my linking here directly!

I think it's a fix worth having the option for. I would also broadly suggest, Django's codebase is updated much more frequently than dbtemplates's is ( as of this writing, the last release was nearly two years ago ). So it probably makes sense to fork dbtemplates, and maybe pipenv install from a version control system as laid out here ( or hey, propose a PR to the project directly; I'd 👍 it ). Patching the smaller package probably helps with the principle of least astonishment, but I could be wrong on that. 💁‍♂️

I'm probably going to make a run of addressing this, but spitballing a bit myself / capturing my thoughts in case I don't get to it soon — I think even just patching this line in dbtemplates/loader.py to fall back more intelligently is probably the path of least resistance, maybe in cooperation with some of this middleware to store the current request. e.g.:

from django.core.exceptions import ImproperlyConfigured
from django_middleware_global_request.middleware import get_request

# ...

   try:
      site = Site.objects.get_current()
   except ImproperlyConfigured:
      request = get_request()
      site = Site.objects.get_current(request=request)

   print("and Bob's your uncle.")
     

Thank you for looking so far into this! Will follow up here as I get some working software ready. :D

@bahoo
Copy link

bahoo commented Sep 14, 2020

Will give this a few days for more changes before I press the issue, but for anyone curious: https://github.com/bahoo/django-dbtemplates

@mpasternak
Copy link
Contributor

Hi @bahoo , from what I understand, you happen to have a non-SITE_ID dependent branch of dbtemplates? Would you mind submitting a PR against the current codebase?

@guettli
Copy link

guettli commented Sep 30, 2022

This could be solved via https://pypi.org/project/django-middleware-global-request/

With a global request, the code could always use Site.objects.get_current(request)

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

4 participants