jazzband / django-downloadview

Serve files with Django.
https://django-downloadview.readthedocs.io
Other
384 stars 57 forks source link

Migrating from django-sendfile #184

Closed dismine closed 2 months ago

dismine commented 3 years ago

Hi.

I have been using django-downloadview version 2.1.1. For my project I have migrated from django-sendfile. But seems your instruction doesn't work for me.

Symptoms. When I download a big file from a website the process terminates after some time. The reason for this is gunicorn worker timeout. Then I realized that I don't serve my files by nginx even if it must to.

It seems for me the code logic is broken. sendfile is just a shortcut for PathDownloadView. Which according to documentation "serve a file using filename". It means a file object will not have the url attribute.

Issue number one.

RealDownloadMiddleware.is_download_response incorrectly handles absences of the url attribute. Standard Python approach "easier to ask for forgiveness than permission" doesn't work here. If we get exception on checking the url we will not check the name attribute. This is exactly what happens in my case.

Right now the method looks like this

    def is_download_response(self, response):
        """Return True for DownloadResponse, except for "virtual" files.

        This implementation cannot handle files that live in memory or which
        are to be dynamically iterated over. So, we capture only responses
        whose file attribute have either an URL or a file name.

        """
        if super(RealDownloadMiddleware, self).is_download_response(response):
            try:
                return response.file.url or response.file.name
            except AttributeError:
                return False
            else:
                return True
        return False

I think, it must look like this

    def is_download_response(self, response):
        """Return True for DownloadResponse, except for "virtual" files.

        This implementation cannot handle files that live in memory or which
        are to be dynamically iterated over. So, we capture only responses
        whose file attribute have either an URL or a file name.

        """
        if super(RealDownloadMiddleware, self).is_download_response(response):
            return hasattr(response.file, 'url') or hasattr(response.file, 'name')
        return False

Issue number two. FIXED

Since we don't have a url we must rely on an attribute source_dir. But to define it you have to be very careful. Only while writing this issue I have realized that I can set up it by replacing source_url by source_dir in DOWNLOADVIEW_RULES. In documentation, you have only "Adapt your settings as explained in Configure". It is very easy to make a copy past error.

dismine commented 3 years ago

Should I send a PR?

Archmonger commented 2 years ago

@dismine Yes, this seems PR'able

tari commented 2 months ago

Fixed by #204.