Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#10497 closed (fixed)

Added storage time methods to Storage and FileSystemStorage classes

Reported by: HuCy Owned by: tobias
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 6 years ago.
storage-stat.2.diff (874 bytes) - added by HuCy 6 years ago.
storage_times.diff (6.1 KB) - added by steph 5 years ago.
Storage backends with time support.
storage_times.2.diff (7.3 KB) - added by steph 5 years ago.
More tests.
storage_times.3.diff (7.3 KB) - added by steph 5 years ago.
Renamed Datetime to match the class name.

Download all attachments as: .zip

Change History (22)

Changed 6 years ago by HuCy

Changed 6 years ago by HuCy

comment:1 Changed 6 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design 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 6 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 6 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 6 years ago by IanLewis

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 5 years ago by steph

  • Component changed from Core framework to File uploads/storage
  • Triage Stage changed from Design decision needed to Unreviewed
  • Version 1.0 deleted

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 5 years ago by steph

Storage backends with time support.

comment:6 Changed 5 years ago by tobias

  • Owner changed from nobody to tobias
  • Status changed from new to assigned

comment:7 Changed 5 years ago by tobias

  • milestone set to 1.3
  • Summary changed from Added 'stat' method to class Storage and FileSystemStorage to Added storage time methods to Storage and FileSystemStorage classes
  • Triage Stage changed from Unreviewed to Ready for checkin
  • Version set to 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 5 years ago by apollo13

  • Triage Stage changed from Ready for checkin to Design decision needed

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

comment:9 Changed 5 years ago by tobias

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 5 years ago by apollo13

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 5 years ago by Alex

  • Triage Stage changed from Design decision needed to Accepted

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 5 years ago by Alex

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 5 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 5 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 5 years ago by steph

More tests.

Changed 5 years ago by steph

Renamed Datetime to match the class name.

comment:15 Changed 5 years ago by Alex

  • Triage Stage changed from Accepted to Ready for checkin

comment:16 Changed 5 years ago by jezdez

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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