Opened 7 years ago

Closed 10 days ago

#11228 closed Cleanup/optimization (wontfix)

FieldFile with check if file exists, and don't raise errors by default

Reported by: Hersonls Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Simon Litchfield Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Many people ask and also has the need to verify that the file is sent to the server is really in its place.

This need arises because they are made unnecessary requests for lack of the file in media server and also for reasons of aesthetics, because the lack of image files or result in a different box with alt attribute, or result in a 404 not found.

The class FieldFile not give the developer the possibility to check if file exists and that is the developers create a method in a class model which will check if file exists on the media server or not.

I created a patch where it created a method for class FieldFile call is_exists which checks if the file exists. This method is valid for all inheritance of FieldFile class.

For example,

{% if object.file.is_exists %}<a href="{{ MEDIA_URL }}{{ object.file }}">download this file</a>{% endif %}
{% if object.image.is_exists %}<img src="{{ MEDIA_URL }}{{ object.image }}" alt="My image" />{% endif %}

Attachments (3)

file.py.diff (676 bytes) - added by Hersonls 7 years ago.
Fix the patch
file2.txt (11 bytes) - added by anonymous 4 years ago.
file2.2.txt (11 bytes) - added by anonymous 4 years ago.

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by Hersonls

Attachment: file.py.diff added

Fix the patch

comment:1 Changed 7 years ago by dc

Needs documentation: set
Needs tests: set
Patch needs improvement: set

If path is stored in database and actual file not exists then data is corrupted and better not create workarounds but fix it immediately.

Anyway os.path.exists will not work with non-local file systems better use self.storage.exists(self.name).

comment:2 Changed 7 years ago by Chris Beaven

Triage Stage: UnreviewedDesign decision needed

And "is_exists" seems ugly. Why not just "exists"?

But yeah, I'm dubious as whether this functionality is important enough to include in the api.

comment:3 Changed 5 years ago by Julien Phalip

Severity: Normal
Type: New feature

comment:4 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset
Resolution: wontfix
Status: newclosed
UI/UX: unset

Like Chris, I doubt this would be generally useful. If you're losing files, you're got bigger problems than broken links in your templates!

Furthermore, it's trivial to subclass FileField to add this method if you need it.

Changed 4 years ago by anonymous

Attachment: file2.txt added

Changed 4 years ago by anonymous

Attachment: file2.2.txt added

comment:5 Changed 11 days ago by Simon Litchfield

Cc: Simon Litchfield added

Sorry to re-open, but the notion that an error is raised when attempting to access attributes on a missing file is a wildly impractical default.

It assumes that "everyone at all times will have a full, up-to-date copy of the media folder, that is exactly in sync with the database they're running against".

If you're coding in a little bubble, eg pre-launch, against some bollocks test data-- sure, lets raise errors and annoy ourselves. But in the "real world" this is often/usually not the case and means you have to move around massive media libraries and databases just to work on the site at all, without resorting to temporarily commenting stuff out or hacking up a custom field.

The default should be to return None, overridable through the field/model definition (eg silent=False). And there should be an exists() method too, why not? @aaugustin your presumption above "that you have bigger problems" is totally irrelevant in many production scenarios.

comment:6 Changed 11 days ago by Simon Litchfield

Resolution: wontfix
Status: closednew

comment:7 Changed 11 days ago by Simon Litchfield

Easy pickings: set
Type: New featureCleanup/optimization

comment:8 Changed 11 days ago by Simon Litchfield

Summary: FieldFile with check if file exists.FieldFile with check if file exists, and don't raise errors by default

comment:9 Changed 10 days ago by Tim Graham

Easy pickings: unset
Resolution: wontfix
Status: newclosed

Hi, the correct procedure to reopen a ticket that's closed as wontfix is to start a discussion on the DevelopersMailingList.

In this case, I'm not sure how related your ideas are to the original ticket. Please try to include a demonstration of the behavior that's problematic. We have what sounds like a similar situation for djangoproject.com. The CorporateMember model has an ImageField for the logo, however, I haven't seen any crashes in Django when those files don't exist.

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