Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#17744 closed Bug (fixed)

override_settings has no effect on FileSystemStorage()

Reported by: mvt Owned by: claudep
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 claudep 2 years ago.
Reset default storage with setting_changed

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by ramiro

  • 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 3 years ago by mvt

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 3 years ago by aaugustin

  • Resolution set to duplicate
  • Status changed from new to closed

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 2 years ago by claudep

  • Resolution duplicate deleted
  • Status changed from closed to reopened

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

comment:5 Changed 2 years ago by claudep

  • Owner changed from nobody to claudep
  • Severity changed from Release blocker to Normal
  • Status changed from reopened to new
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

Changed 2 years ago by claudep

Reset default storage with setting_changed

comment:6 Changed 2 years ago by claudep

  • Has patch set

mvt, would this patch address your issue?

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

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

In 9a02851340c5c30b8e2c174783cd86d5cab7ab81:

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

comment:8 Changed 2 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