Opened 12 months ago

Closed 10 months ago

Last modified 10 months ago

#23376 closed Cleanup/optimization (fixed)

Inconsistent documentation about required Storage methods

Reported by: jaap3 Owned by: brwr
Component: Documentation Version: master
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 Changed 12 months ago by bmispelon

  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/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 Changed 12 months ago by evildmp

  • 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 Changed 12 months ago by masterfung

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 12 months ago by masterfung (previous) (diff)

comment:4 Changed 12 months ago by masterfung

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

comment:5 Changed 12 months ago by timgraham

  • 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 Changed 11 months ago by EvilDMP

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 Changed 10 months ago by brwr

  • Owner changed from masterfung to brwr

comment:8 Changed 10 months ago by MarkusH

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Patch looks good to me.

comment:9 Changed 10 months ago by James Brewer <james@…>

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

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 Changed 10 months ago by Daniele Procida <daniele@…>

In d968bd5258438694b000b347a482ddcd33a9b395:

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

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

comment:11 Changed 10 months ago by Tim Graham <timograham@…>

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