Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#23376 closed Cleanup/optimization (fixed)

Inconsistent documentation about required Storage methods

Reported by: Jaap Roes Owned by: James Brewer
Component: Documentation Version: dev
Severity: Normal Keywords: custom storage documentation, afraid-to-commit
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

https://docs.djangoproject.com/en/dev/howto/custom-file-storage/ mentions the following:

[...] but you must implement the following methods:

  • Storage.delete()
  • Storage.exists()
  • Storage.listdir()
  • Storage.size()
  • Storage.url()

Yet https://docs.djangoproject.com/en/dev/ref/files/storage/#the-storage-class says that delete, listdir, size and url may raise NotImplementedError. Which is it?

Change History (11)

comment:1 by Baptiste Mispelon, 10 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Hi,

Indeed, this appears to be inconsistent.
A quick look at the history of the files seem to indicate that the "topic"-style documentation is older than the reference documentation so it might have gotten out of sync.

I'm not familiar with the storage API but it seems that must could be replaced by should here.

Thanks.

comment:2 by Daniele Procida, 10 years ago

Keywords: afraid-to-commit added

I've marked this ticket as especially suitable for people following the ​Don't be afraid to commit tutorial at the DjangoCon US 2014 sprints. If you're tackling this ticket, please don't hesitate to ask me for guidance if you'd like any, either at the sprints themselves, or here or on the Django IRC channels, where I can be found as EvilDMP.

comment:3 by Johnny Hung, 10 years ago

I would love to contribute to this as a beginner. I am not able to attend the sprints but I would love the feedback.

Pull request: https://github.com/django/django/pull/3181

Please let me know if there are any questions.

Last edited 10 years ago by Johnny Hung (previous) (diff)

comment:4 by Johnny Hung, 10 years ago

Owner: changed from nobody to Johnny Hung
Status: newassigned

comment:5 by Tim Graham, 10 years ago

Has patch: set
Patch needs improvement: set

I think it would be good to add details about things that might break if you don't implement each method.

comment:6 by Daniele Procida, 9 years ago

I've marked this ticket as especially suitable for people following the ​Don't be afraid to commit tutorial at the PyCon Ireland 2014 sprints. If you're tackling this ticket, please don't hesitate to ask me for guidance if you'd like any, either here or on the Django IRC channels, where I can be found as EvilDMP.

comment:7 by James Brewer, 9 years ago

Owner: changed from Johnny Hung to James Brewer

comment:8 by Markus Holtermann, 9 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Patch looks good to me.

comment:9 by James Brewer <james@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 332706eaa02d191118966801ae4f4607a767c0eb:

Fixed #23376 -- Made documentation about required Storage methods
consistent.

The following methods should be implemented, but are not required:

  • Storage.delete()
  • Storage.exists()
  • Storage.listdir()
  • Storage.size()
  • Storage.url()

Updated documentation to reflect this fact and give a couple of examples
where some methods may not be implemented. Add a warning that not
implementing some methods will result in a partial (possibly broken)
interface.

Ticket: https://code.djangoproject.com/ticket/23376

comment:10 by Daniele Procida <daniele@…>, 9 years ago

In d968bd5258438694b000b347a482ddcd33a9b395:

Merge pull request #3459 from brwr/brwr/django-23376

Fixed #23376 -- Make documentation about required Storage methods consistent.

comment:11 by Tim Graham <timograham@…>, 9 years ago

In 74d0311d6b1fb3ab16072d5fad9de8b996e8af1c:

[1.7.x] Fixed #23376 -- Clarified that certain Storage methods should be implemented but are not required.

Backport of 332706eaa0 from master

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