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

Migrating from django-sendfile #184

Closed
dismine opened this issue May 16, 2021 · 3 comments
Closed

Migrating from django-sendfile #184

dismine opened this issue May 16, 2021 · 3 comments

Comments

@dismine
Copy link

dismine commented May 16, 2021

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
Copy link
Author

dismine commented May 20, 2021

Should I send a PR?

@Archmonger
Copy link

@dismine Yes, this seems PR'able

@tari
Copy link
Contributor

tari commented Jul 31, 2024

Fixed by #204.

@tari tari closed this as completed Jul 31, 2024
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

3 participants