Pass the request down to the Sitemap callable

[rm_]

It would be handy if the Sitemap would know a bit more about the environment so pass down to it the request. This is useful for me because i serve the same app under different domains and i'd like to serve a bit different sitemap. This could be useful too if you want to serve a different sitemap depending on the REMOTE_ADDR or HTTP_USER_AGENT.

Diffed against r17606.

Tests looks ok:

$ PYTHONPATH=. tests/runtests.py --settings=test_sqlite
Creating test database for alias 'default'...
Creating test database for alias 'other'...
[snip]

---

Ran 4678 tests in 627.803s

OK (skipped=94, expected failures=2)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

[rm_]

Diffed against r17606

[Aymeric Augustin]

This patch doesn't change anything to the public API. It only makes it easier for developers to write subclasses of Sitemap (which isn't officially supported).

If we decided to make this change, it would have to be tested. Otherwise, it could be refactored away by mistake. We could also document more parts of Sitemap, making them public APIs.

---

However, I'm not sure this patch is the best solution to the original problem. If you "serve the same app under different domains", you must be using different sites, which means different settings.SITE_ID. IMO you should just change the output of your sitemaps based on the current site (for the "different domains" part), or on another setting.

Regarding:

This could be useful too if you want to serve a different sitemap depending on the REMOTE_ADDR or HTTP_USER_AGENT.

nobody's asked for that yet, I invoke the YAGNI principle.

Tentatively closing "needsinfo", please reopen if that solution doesn't work.

[me@…]

Bump.

I'm using an own set request variable; request.LANGUAGE (based on TLD) to select the content on my site, for example Foo.objects.filter(language=request.LANGUAGE).

Sure this could be set with the Sites framework and use a Site for the lookup instead, however this means I need to run a wsgi process for each language (with different SITE_ID), if I were to add 20 languages for my site with different content I then need to run 20 instances of django.

I'd prefer to add the request object to the Sitemap object instead.

I'm reopening, please consider this again.

[rm_]

Replying to me@…:

I'm using an own set request variable; request.LANGUAGE (based on TLD) to select the content on my site, for example Foo.objects.filter(language=request.LANGUAGE).

Sure this could be set with the Sites framework and use a Site for the lookup instead, however this means I need to run a wsgi process for each language (with different SITE_ID), if I were to add 20 languages for my site with different content I then need to run 20 instances of django.

Share the same sentiments about SITE_ID

I'm reopening, please consider this again.

Care to respin the patch with the tests aaugustin asked for?

[pdina]

I sent a pull request on github, the patch contains test and docs.

​https://github.com/django/django/pull/2371

[Aymeric Augustin]

Comments 2 and 3 look like they belong to #15089. The problem here isn't sitemaps, it's multi-tenancy -- and it's a hard problem.

[Aymeric Augustin]

I can't say what the proper solution is, but I'm afraid hardcoding a sitemap-specific solution will only make proper multi-tenancy harder. I think this requires a discussion on the DevelopersMailingList. See also #22231.

[Aymeric Augustin]

Err. Didn't mean to close it.

[rm_]

Replying to aaugustin:

I can't say what the proper solution is, but I'm afraid hardcoding a sitemap-specific solution will only make proper multi-tenancy harder. I think this requires a discussion on the DevelopersMailingList. See also #22231.

Thanks for your comment Aymeric. Proper multi tenancy in django would be very cool (at the time liked a lot alex gaynor's talk [1] about getting the application state from the request) but just adding an attribute to a class it's fixing the issue for at least a couple of us in quite an unobtrusive way.

[1] ​https://speakerdeck.com/alex/take-2-if-i-got-to-do-django-all-over-again

[rm_]

Replying to aaugustin:

I can't say what the proper solution is, but I'm afraid hardcoding a sitemap-specific solution will only make proper multi-tenancy harder. I think this requires a discussion on the DevelopersMailingList. See also #22231.

So tried to open a discussion on mailing list few weeks ago without much success. The side effect though is that i'm very much interested in the patch [2] attached to #15089 [3] which make this #17802 ticket obsolete. Aymeric, any chance you can review the patch?

[1] ​https://groups.google.com/forum/#!topic/django-developers/ORo_28B_VIA
[2] https://code.djangoproject.com/attachment/ticket/15089/0001-Make-settings.SITE_ID-optional-for-django.contrib.si.patch
[3] https://code.djangoproject.com/ticket/15089

[Tim Graham]

If this ticket will be obsoleted by #15089, then I guess we can close this as a duplicate.