Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#10497 closed (fixed)

Added storage time methods to Storage and FileSystemStorage classes

Reported by: HuCy Owned by: Tobias McNulty
Component: File uploads/storage Version: 1.2
Severity: Keywords: storage
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The included patch implements a stat method for Storage and FileSystemStorage in django.core.files.storage

Background:
We implemented a storage engine for CouchDB in which catalog of products are stored as pdf documents.
The catalogs are re-generated depending on their age - which was easy when the default FileSystemStorage was
used, but more complicated using a custom storage engine that does not use a filesystem backend.

Adding a stat method to the Storage class makes it easier to gain the required information, especially for
derived classes as described above.

Attachments (5)

storage-stat.diff (1.4 KB) - added by HuCy 8 years ago.
storage-stat.2.diff (874 bytes) - added by HuCy 8 years ago.
storage_times.diff (6.1 KB) - added by steph 6 years ago.
Storage backends with time support.
storage_times.2.diff (7.3 KB) - added by steph 6 years ago.
More tests.
storage_times.3.diff (7.3 KB) - added by steph 6 years ago.
Renamed Datetime to match the class name.

Download all attachments as: .zip

Change History (22)

Changed 8 years ago by HuCy

Attachment: storage-stat.diff added

Changed 8 years ago by HuCy

Attachment: storage-stat.2.diff added

comment:1 Changed 8 years ago by Jacob

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

I'm not sure this is appropriate for a generic backend interface: how would you implement stat() for S3, for example?

comment:2 Changed 8 years ago by anonymous

An HTTP HEAD request would give you some valuable information:

HEAD /testmeplease-us/10mb.bin HTTP/1.1
Host: s3.amazonaws.com

HTTP/1.1 200 OK
Date: Sat, 14 Mar 2009 21:13:16 GMT
Last-Modified: Fri, 30 Nov 2007 19:36:55 GMT
ETag: "1a54772a48158d16a8ede6ef10b4ea25"
Content-Type:
Content-Length: 10485760
Server: AmazonS3

comment:3 Changed 8 years ago by HuCy

Parsing the HTTP header is a good idea.
I think for every storage engine, there's some way to provide the data.
For example for the CouchDB storage engine, we implemented the stat method by adding fields to the CouchDB documents.

comment:4 Changed 7 years ago by Ian Lewis

Unfortunately none of the things like protection bits, inode no and other disk related information. The data returned by os.stat also will be different for different platforms. See http://docs.python.org/library/os.html#os.stat

For instance on POSIX systems you won't be able to get the document creation time since it's not included in the file system metadata. And you can already get the modified time from the "modified" method.

I guess you could do something like add custom http headers to your files in amazon S3 or your backend to implement the stat command but I don't think it would really give you useful information.

comment:5 Changed 6 years ago by steph

Component: Core frameworkFile uploads/storage
Triage Stage: Design decision neededUnreviewed
Version: 1.0

I think, added a full os.stat would be hard to implement on non-filesystem based storage backends. Because of this fact, i propose to just add three new methods to get the accessed time, created time and modified time.

By splitting the times up in multiple methods, a storage backend can decide what times are supported.

I'll attach a patch with tests and docs to this ticket.

Changed 6 years ago by steph

Attachment: storage_times.diff added

Storage backends with time support.

comment:6 Changed 6 years ago by Tobias McNulty

Owner: changed from nobody to Tobias McNulty
Status: newassigned

comment:7 Changed 6 years ago by Tobias McNulty

milestone: 1.3
Summary: Added 'stat' method to class Storage and FileSystemStorageAdded storage time methods to Storage and FileSystemStorage classes
Triage Stage: UnreviewedReady for checkin
Version: 1.2

Patch applied and tests pass. The patch includes docs, which look good. Seems like a reasonable change. It's a small change and would be useful for, among other things, cases like thumbnail creation & maintenance. Marking it RFC.

comment:8 Changed 6 years ago by Florian Apolloner

Triage Stage: Ready for checkinDesign decision needed

Plz don't move your own tickets to RFC, aside from that the DDN from Jacob still aplies.

comment:9 Changed 6 years ago by Tobias McNulty

The proposed change is completely different than the one Jacob DDN'ed, and also addresses his concern. Additionally, this isn't my ticket, nor did I create the patch. That said, I'm leaving it "DDN" because I have no idea who "apollo13" is.

comment:10 Changed 6 years ago by Florian Apolloner

Hmm; you might be right, but it was my impression from the contribution docs, that you should let a core-dev move it out of DDN. Apart from that the docs tell us not to move tickets to RFC, unless they are trivial; features don't fall into that category from what I can tell. I guess the best way to address this would be to get a core dev look over it. Sry if I caused any inconvenience.

comment:11 Changed 6 years ago by Alex Gaynor

Triage Stage: Design decision neededAccepted

I'm marking as Accepted on the basis of the fact that it's a reasonable feature, and it addresses jacob's concerns.

comment:12 Changed 6 years ago by Alex Gaynor

Couple nits to pick with the patch. a) Can we switch to using the :function: syntax in the storage docs, better metadata is always useful. b) I'd like to strengthen the asserts in the tests to also test that the times are all within the last 2 seconds or something (ideally we'd verify exact times, but that's hard). Otherwise I think it's fine.

comment:13 Changed 6 years ago by steph

I think the asserts a strong enough because we already test against the exact times, or do you mean anything else? Regarding the docs, I think this should go to a different ticket because the complete storage docs should be rewritten to use the function: syntax.

comment:14 Changed 6 years ago by steph

OK, just added some asserts to make sure, the files are new and times are not older than 2 seconds.

Changed 6 years ago by steph

Attachment: storage_times.2.diff added

More tests.

Changed 6 years ago by steph

Attachment: storage_times.3.diff added

Renamed Datetime to match the class name.

comment:15 Changed 6 years ago by Alex Gaynor

Triage Stage: AcceptedReady for checkin

comment:16 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: assignedclosed

(In [14012]) Fixed #10497 -- Added a few time-related methods to the storage API. Thanks for the report and patch to Stephan Jaekel.

comment:17 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top