sendfile() accepts file wrapper instead of filename

[benoitbryon]

As of version 0.3.2, sendfile() function takes a filename as input, and checks that this file exists using os.path.exists.
See

django-sendfile/sendfile/__init__.py

Line 48 in 298f632

if not os.path.exists(filename):

This implementation limits sendfile() usage to files that live on local filesystem.

What about accepting file wrappers instead of filenames? such as FieldFile (file wrapper for FileField).
This would make it possible to serve files from various locations: local filesystem, storages, URL, in-memory files...

[johnsensible]

I'm not sure this will work. sendfile is just a very thin wrapper that normally just sets a header to get nginx, apache etc to do the real work - which is much better for performance. They expect a "real" file path and won't be able to deal with a file wrapper (as that's a python concept).

The sendfile wrapper is merely a layer to smooth over the differences between the xsendfile/accellredirect stuff (as well as providing something to use during development).

If you want to use a file wrapper you might as well just return a regular Django HttpResponse with the contents of the file.

[benoitbryon]

This "file wrapper" feature is supported by django-downloadview version 1.1. It works ;)

If you want to use a file wrapper you might as well just return a regular Django HttpResponse with the contents of the file.

That's the way it is implemented in django-downloadview: the view returns some DownloadResponse (inherits from StreamingHttpResponse), which can be intercepted by middlewares (x-accel, x-sendfile...) before it is loaded into memory.

In #9 you said:

I don't think it really makes sense to merge the projects, but it would possibly make sense for django-downloadview to use django-sendfile, as django-sendfile is really just a small building block.

As of django-sendfile version 0.3.2 and django-downloadview version 1.1, django-downloadview has poor interest using django-sendfile, because django-downloadview can serve files that don't live in local filesystem. It could use django-sendfile to serve local files, but would have to re-implement (duplicate) most things to support remote files.

That said, I asked here because I feel that it would be useful for both projects to exchange some features. This "file wrapper" feature is one example that could be moved from django-downloadview to django-sendfile. Then, perhaps, django-downloadview could use django-sendfile.

But feel free to discard this issue if you don't want to change sendfile() signature ; I would understand this opinion :)

[benoitbryon]

Note: I'll be at europython (Florence) and djangocon.eu (Warsaw) this year... if you or some django-sendfie contributor are there too and would like to discuss/sprint in real life ;)

[johnsensible]

Hi Benoit,

sendfile is really just concerned with local files - it's intended to get apache/nginx to use the OS's sendfile (http://linux.die.net/man/2/sendfile) call for maximum efficiency. It's purely an optimisation, though when serving file that is several Mb (or possibly even several Gb) in size it can make a difference.

Supporting file wrappers would not allow for this optimisation, which would defeat the purpose of using the library in the first place.

The scope of sendfile is much smaller than what you seem to be trying to achieve with django-downloadview. It really is just meant to be a very small, very simple library to paper over the differences between using nginx or xsendfile (and the django dev server).

In #9 I wasn't suggesting that you should use django-sendfile, just that you could - just detect whether you're serving a real file or a wrapper and call sendfile if it's a real file. If done right I don't imagine you'd need much/any duplication in your code - only the point at which you return a HttpResponse should changed really. It looks like you already support nginx etc anyway?

So yeah, I guess it's a polite no from me on supporting file wrappers ...

cheers,

John

p.s. sorry, won't be going to either of those conferences I'm afraid.

[benoitbryon]

I guess it's a polite no from me on supporting file wrappers

Ok, not a big deal ;)
Let's close the issue.

It looks like you already support nginx etc anyway?

Yes. Only nginx for now. And that's a reason why I'm looking at using django-sendfile and its backends.

sendfile is really just concerned with local files

That's the main point imho.
Django-downloadview supports both (local) filenames and (remote or local) URL. The latter relies on nginx's configuration, i.e. using "root" for local files, whereas using "proxy_pass" for URL.

won't be going to either of those conferences

Maybe another day, in another place...