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

DownloadView and DownloadResponse use some file wrapper #23

Closed
benoitbryon opened this issue Dec 11, 2012 · 4 comments
Closed

DownloadView and DownloadResponse use some file wrapper #23

benoitbryon opened this issue Dec 11, 2012 · 4 comments

Comments

@benoitbryon
Copy link
Collaborator

In download views, we want to instanciate a download response.
We currently compute file attributes (size, name, ...) and pass it to the response constructor.
The more attributes (see #21), the more the response class gets complicated.
Moreover we would like these attributes (see #22) to be lazy.
We could implement some methods that support storages (and thus, FileField and ImageField).
But then, what if someone wants to change the behaviour? He would have to override the response class.

The response class should have some "file" (or whatever the name) attribute, and handle it via some API. Could sound like "the response's file attribute is an object with url, filename, basename, size... attributes".
Then it would make it possible to have various implementations for this "file wrapper". One would use Django storages.

Could look like django.db.models.fields.files.FieldFile, but not tied to models.

Could help #5, #21 and #22.

List of attributes for the file wrapper:

  • name: absolute filename in filesystem
  • url: URL where file contents live
  • size: in bytes
  • mime_type
  • encoding
  • charset
  • modification_time
  • content: iterator over file contents

Are these attributes needed?

  • media_root: typically storage's location
  • relative_filename: filename relative to media_root
  • is_virtual: True if the file isn't on some disk, i.e. Django/Python is to compute file contents. Should be True if filename (and maybe URL too) is empty. Could also be "is_persistent", i.e. will the file live after Django processed it.
@benoitbryon
Copy link
Collaborator Author

basename is an attribute of the download response. Can be different from file's name.

@benoitbryon
Copy link
Collaborator Author

Some examples...

Use case: stream a file which exists on disk.

  • django.core.files.File is a suitable wrapper.
  • provides "name" attribute
  • provides iterator over file contents.

Use case: a file that lives in some model, using a storage.

  • model's FieldFile is a suitable wrapper
  • optionally provides "url" attribute, depending on storage implementation
  • optionally provides "name" attribute, depending on storage implementation
  • provides iterator over file contents.

Use case: generate and stream a CSV file without writing it to disk:

  • needs a suitable file wrapper
  • doesn't provide url or name
  • provides iterator over file contents

=> The iterator over file contents seems to be the only mandatory requirement.

=> name and url are optional. Even if they seem recommended for server optimizations (Django could write in a temporary file, but this is not the best optimization).

=> size is recommended, but optional. If "name" is provided, then size can be computed with os.stat. If URL is provided, size could be useful, but in case of server optimizations, the server may be able to add the content-length header. If the file is a virtual one (dynamic streaming), then it seems difficult to provide a size before the whole file contents is iterated. Does the lack of "Content-Length" compromise the download?

=> content_type, mime_type, charset and maybe some other attributes are optional: they can be computed by the response class. However, if they are provided, we should use them, because it allows optimizations and customizations.

=> modification_time seems useful, but could be optional (could default to "always new").

@benoitbryon
Copy link
Collaborator Author

Django's storage API provides "modified_time" (not "modification_time"), "accessed_time" and "created_time".

@benoitbryon
Copy link
Collaborator Author

In Django's storage API (and thus FieldFiles), file wrappers have:

  • name: filename which could be relative or absolute. It is a value used to open the file (with open() in case of a local file, with the storage in case of a FieldFile).
  • path: absolute local filename. Not provided if storage doesn't support usage of Python builtin open()

=> The difference seems important. Let's keep this design.

benoitbryon added a commit to peopledoc/django-downloadview that referenced this issue Dec 13, 2012
…rapper to download response. The file wrapper encapsulates file attributes such as name, size or URL (introduced URL).
benoitbryon added a commit to peopledoc/django-downloadview that referenced this issue Feb 6, 2013
…DownloadView is to replace an use case of former DownloadView.
benoitbryon added a commit to peopledoc/django-downloadview that referenced this issue Feb 6, 2013
…torageDownloadView is to replace an use case of former DownloadView.
benoitbryon added a commit to peopledoc/django-downloadview that referenced this issue Feb 6, 2013
…r basic support of in-memory (StringIO) or other generated files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant