Sitemap implementation means it cannot be updated automatically

[tenkujin]

The Sitemap class uses an attribute _paginator which stores all the URLs. The method items() is thus called only once for the creation of the paginator, and subsequent invocations of the sitemap view do not call it. If the items() method is meant to return dynamically changing items (which is a common case), changes are not reflected in the sitemap. The only way to fix this is to either restart Django (not acceptable), override the get_urls() method and update the _paginator (not nice), or re-implement the Sitemap object to not have such semantics (violates DRY).

Since this paginator is a hack to accommodate Google's arbitrary 50k URL limit, I think it would be best to have a Sitemap which can be updated at runtime, and an optional GoogleSitemap for this particular search engine.

[Julian Bez]

"subsequent invocations of the sitemap view do not call it"

Are you sure? AFAIK Django treats every request as a completely new one.

[Arthur Koziel]

The 50k limit is not arbitrary. Google, like MSN and Yahoo, use the sitemaps.org standard which specifies the 50k limit.

See ​http://sitemaps.org/protocol.php#index for more Information.

[tenkujin]

Replying to arthurk:

The 50k limit is not arbitrary. Google, like MSN and Yahoo, use the sitemaps.org standard which specifies the 50k limit.

See ​http://sitemaps.org/protocol.php#index for more Information.

Still, by storing the paginator in the self._paginator field and not providing a way to clear it (signals? cache? method?) means we need to fiddle with the internals of Django sitemaps simply to have dynamic sitemaps. I think the pagination logic should somehow check to see whether the query set it uses has changed since the last time the paginator was created and assigned to _paginator

[Denis Martinez <deuns.martinez@…>]

proposed solution for dynamic sitemaps

[Denis Martinez <deuns.martinez@…>]

Here's a solution to the problem, which I'm currently using on my website.
I applied the above patch to django 1.1 and I modified urls.py to look like this:

urlpatterns = patterns(
    '',
    url(r'^sitemap.xml$', 'django.contrib.sitemaps.views.index', {'sitemaps': get_sitemaps}, name='sitemap_index'),
    url(r'^sitemap-(?P<section>.+)\.xml$', 'django.contrib.sitemaps.views.sitemap', {'sitemaps': get_sitemaps}, name='sitemap'),
    )

where get_sitemaps is a function which returns the sitemaps directory.

def get_sitemaps():
    sitemaps = {}
    sitemaps['blogs'] = BlogSitemap
    ......
    return sitemaps

Using this method my sitemaps are instanciated on every request, and I get a new paginator, and expected results.

[Greg Thornton]

The approach in sitemaps.patch invalidates the current documentation, and doesn't take into consideration the fact that an previously instantiated Sitemap instance may be used instead of a subclass. I don't know if the pagination should really be handled by the Sitemap class at all, since it seems like a more view specific attribute. Since ditching the class based pagination would require a lot of view changes, I've been using a subclass of Sitemap whose paginator attribute is instantiated each time it is accessed. When using the current sitemap views, the paginator will only be accessed once, so only one paginator is initialized for each request (luckily).

For reference, I believe this bug only shows up if you add a Sitemap instance to the sitemaps dict. Using classes only shouldn't be a problem since the short-lived instance will be recreated each time the view is called.

Should we dumb down Sitemap and move pagination into the views? It seems like that would be more flexible.

[Christopher Medrela]

Dynamic paginator creation, checking for DB changes and time limit

[Christopher Medrela]

We check time to life of paginator. If it is older than 10 minutes (Sitemap.reload_delta), we force recreation of new paginator. If it is older than 1 minute (Sitemap.counter_delta) we check whether number of items in DB has changed and recreate paginator if so.

[Christopher Medrela]

Added documentation to previous patch, moved checking code to separate method.

[Grzegorz Nosek]

The Paginator object is needlessly cached for the whole lifetime of the Sitemap object, which may be the whole lifetime of the server.

After discussing with jezdez, we considered the potential performance impact (evaluating len(sitemap.items()) at every request acceptable for the time being. Fixing this properly would require Paginators to lazily determine the number of pages only when actually needed, which is quite outside of the scope of this ticket.

[Jannis Leidel]

In [17468]:

Fixed #10793 -- Stopped caching paginator instances in sitemap classes to prevent stale sitemaps. Thanks, gnosek, krzysiumed and adam_przybyla.