Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#17744 closed Bug (fixed)

override_settings has no effect on FileSystemStorage()

Reported by: Michael van Tellingen Owned by: Claude Paroz
Component: File uploads/storage Version: 1.4-beta-1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since FileSystemStorage() copies the value of settings.MEDIA_ROOT to self.location on init. This means that changes to settings.MEDIA_ROOT have no effect.

I think the best solution here is to listen for the settings_changed signal and update the location value if the default value (which is settings.MEDIA_ROOT) is used. Unfortunately this cannot be solved when passing a custom location value based on the MEDIA_ROOT.

Otherwise this should be clearly documented.

I could create the patch if there is interest for this.

Attachments (1)

17744-1.diff (3.9 KB) - added by Claude Paroz 4 years ago.
Reset default storage with setting_changed

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by Ramiro Morales

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Do you have need to change MEDIA_ROOT during of the life time of a FileSystemStorage instance and get it to react to such changes?.

Note: All this in the context of running tests because modification of settings isn't supoported in other scenarios.

comment:2 Changed 5 years ago by Michael van Tellingen

I hit the problem when using the following construct:

class TestCase(BaseTestCase):
    maxDiff = None

    def setUp(self):
        super(TestCase, self).setUp()

        # Create path to temp dir and recreate it
        temp_media_root = os.path.join(
            tempfile.gettempdir(), 'project-testrun')
        if os.path.exists(temp_media_root):
            shutil.rmtree(temp_media_root)
        os.mkdir(temp_media_root)

        self.override = override_settings(MEDIA_ROOT=temp_media_root)
        self.override.enable()

    def tearDown(self):
        shutil.rmtree(settings.MEDIA_ROOT)
        self.override.disable()

        super(TestCase, self).tearDown()

This makes sure I have a clean media_root for each test, which is a good thing. The documentation led me to believe this should be possible, but going through the source it was pretty easy to see that it is not.

Perhaps a clear warning in the documentation is enough, on the other hand I think something like this should work

comment:3 Changed 5 years ago by Aymeric Augustin

Resolution: duplicate
Status: newclosed

This is a particular instance of the general problem tracked in #17787 (which I just created). Let's keep the more general ticket.

A note was added to the docs in r17597, resolving the "release blocker" aspect.

comment:4 Changed 4 years ago by Claude Paroz

Resolution: duplicate
Status: closedreopened

Reopening, as we didn't address this specific setting in #17787.

comment:5 Changed 4 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Severity: Release blockerNormal
Status: reopenednew
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Changed 4 years ago by Claude Paroz

Attachment: 17744-1.diff added

Reset default storage with setting_changed

comment:6 Changed 4 years ago by Claude Paroz

Has patch: set

mvt, would this patch address your issue?

comment:7 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 9a02851340c5c30b8e2c174783cd86d5cab7ab81:

Fixed #17744 -- Reset default file storage with setting_changed signal

comment:8 Changed 4 years ago by Claude Paroz <claude@…>

In a24ffa52d02998f5082fbc5a461ccdc8004db4a6:

[1.5.x] Fixed #17744 -- Reset default file storage with setting_changed signal

Backport of 9a0285134 from master.

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