django-static-sitemaps icon indicating copy to clipboard operation
django-static-sitemaps copied to clipboard

When using s3 boto storage, path gets prepended to the filename twice

Open lsemel opened this issue 7 years ago • 6 comments

The value of SITEMAP_ROOT_DIR gets prepended to the filename twice. I had do use a custom storage to avoid this happening:

class S3SitemapStorage(S3BotoStorage):
    def __init__(self, *args, **kwargs):
        super(S3SitemapStorage, self).__init__(*args, **kwargs)
        self.location = ''

lsemel avatar Mar 11 '17 19:03 lsemel

Used an alteration of the above to fix this for the normal storage method.

Add to settings.py

STATICSITEMAPS_STORAGE = 'myproject.sitemaps.SitemapStorage'

Add to myproject/sitemaps.py

from django.core.files.storage import FileSystemStorage

class SitemapStorage(FileSystemStorage):
    def __init__(self, *args, **kwargs):
        super(SitemapStorage, self).__init__(*args, **kwargs)
        self.location = ''

As @lsemel says, this fixes the duplication of a custom SITEMAP_ROOT_DIR

digitaltommy avatar Nov 02 '17 16:11 digitaltommy

@digitaltommy Can you make a pull request out of it? It would be appreciated. Thx

xaralis avatar Nov 06 '17 09:11 xaralis

@xaralis sorry just seen this comment - do these pull requests need doing still?

digitaltommy avatar Feb 01 '18 17:02 digitaltommy

It's because SitemapGenerator.storage is initialized with location=conf.ROOT_DIR (https://github.com/xaralis/django-static-sitemaps/commit/5ede702d8f8cd99d7f1987c9a8fd41bf7262060a#diff-3d35d9fe93e9b30725df9b15374bea77R27)

-        storage = _lazy_load(conf.STORAGE_CLASS)()
+        self.storage = _lazy_load(conf.STORAGE_CLASS)(location=conf.ROOT_DIR)

and S3 storage prepends self.location to every path you want to access. (https://github.com/jschneier/django-storages/blob/1.7.1/storages/backends/s3boto.py#L363)

            return safe_join(self.location, name)

Before fixing it, we should find out the original intent of location=conf.ROOT_DIR in the first place. @xaralis

puzzlet avatar Sep 19 '18 12:09 puzzlet

@puzzlet Correct. I've reviewed the code and don't see any specific reason why this line (self.storage = _lazy_load(conf.STORAGE_CLASS)(location=conf.ROOT_DIR)) should have location argument in the call. Simply beacuse when writing using the storage, full path is provided too.

It's probably some old thing that noone noticed. It should be safe to remove.

xaralis avatar Sep 22 '18 08:09 xaralis

still not fixed?

bashu avatar Jun 08 '19 08:06 bashu