Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes for Django 5.1, add support for new STORAGES setting #524

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

domvie
Copy link
Contributor

@domvie domvie commented Aug 17, 2024

Description

  • Add custom get_storage_class to replace the removed method in Django 5.1 and avoid breaking changes
  • Add support for new STORAGES setting using the 'dbbackup' alias.

Example:

STORAGES = {
    "default": {
        "BACKEND": "django.core.files.storage.FileSystemStorage",
        "OPTIONS": ...,
    },
    "dbbackup": {
        "BACKEND": "django.core.files.storage.FileSystemStorage",
        "OPTIONS": {
            "location": "/my/backup/dir/",
        },
    },
}

The previous DBBACKUP_STORAGE and DBBACKUP_STORAGE_OPTIONS are still valid and take precedence.

Fixes #522 and should also fix #523.

Please double check if I missed anything.

What confused me a bit was that for the DB Storage this package always uses the configured DBBACKUP_STORAGE, while the mediabackup storage always uses the default django storage. I kept this behavior to not break anything.

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

Copy link

codecov bot commented Aug 17, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 90.97%. Comparing base (8f1307e) to head (ac00a03).
Report is 2 commits behind head on master.

Files Patch % Lines
dbbackup/settings.py 62.50% 1 Missing and 2 partials ⚠️
dbbackup/storage.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   91.40%   90.97%   -0.44%     
==========================================
  Files          19       19              
  Lines         873      886      +13     
  Branches      157      159       +2     
==========================================
+ Hits          798      806       +8     
- Misses         40       43       +3     
- Partials       35       37       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Archmonger Archmonger marked this pull request as draft August 19, 2024 06:12
@Archmonger
Copy link
Contributor

Converting to draft until tests are added.

@domvie domvie marked this pull request as ready for review August 19, 2024 07:35
@Archmonger
Copy link
Contributor

I'm okay with a breaking change that completely removes the DBBACKUP_STORAGE setting, especially given the mismatch with mediabackup.
Would you like to do that within this PR? Otherwise, it'll be handled in a follow up PR.

@domvie
Copy link
Contributor Author

domvie commented Aug 20, 2024

I think I'd prefer that as another PR. That would also imply dropping support for Django <4.2 and would require a rewrite of some existing tests (e.g. I believe FakeStorage does currently not work for mediabackup etc.)

@Archmonger Archmonger merged commit 8e15220 into jazzband:master Aug 20, 2024
7 checks passed
@rapsli
Copy link

rapsli commented Aug 21, 2024

great. Just ran into this. When will this be released on pip?

@Archmonger
Copy link
Contributor

Will be released today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants