jazzband / django-downloadview

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

Did this module ever close a file? #155

Open filwaline opened 4 years ago

filwaline commented 4 years ago

DownloadMixin need a File object to serve, and open file in get_file method, but I never found a close call(by global text search). Did you close file somewhere? Or Using context manager with?

https://docs.djangoproject.com/en/2.2/topics/files/#the-file-object

If open too many file and never close them, IOError will raise.

IOError: [Errno 24] Too many open files
Natim commented 3 years ago

@filwaline that's an interesting question. Do you have a way to fix this in mind?

filwaline commented 3 years ago

use @contextlib.contextmanager

@contextlib.contextmanager
def get_file(self):
    fileobject = open(...)
    try:
        yield fileobject
    finally:
        fileobject.close()
    try:
        with self.get_file() as f:
            self.file_instance = f

            # Respect the If-Modified-Since header.
            since = self.request.META.get("HTTP_IF_MODIFIED_SINCE", None)
            if since is not None:
                if not self.was_modified_since(self.file_instance, since):
                    return self.not_modified_response(**response_kwargs)
            # Return download response.
            return self.download_response(*response_args, **response_kwargs)

    except exceptions.FileNotFound:
        return self.file_not_found_response()

https://github.com/jazzband/django-downloadview/blob/master/django_downloadview/views/base.py#L144

code not tested, but it should work

Natim commented 3 years ago

Do you mind creating a pull-request with your fix?

filwaline commented 3 years ago

Maybe later