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

fix: Update django-storages options #71

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

fghaas
Copy link
Contributor

@fghaas fghaas commented Feb 20, 2024

The Open edX Quince release comes with a new django-storages version (1.14).

New versions of django-storages have stricter checks for STORAGE_KWARGS. It is no longer allowed to have options from other storage classes, such as base_url, which is irrelevant to the S3 storage class.

The bucket option has also been removed. It has been named bucket_name since django-storages 1.7 (even Open edX Lilac used 1.8, so there are no backward compatibility concerns); 1.10 dropped the bucket alias.

@fghaas
Copy link
Contributor Author

fghaas commented Feb 20, 2024

@CefBoud This is an updated version of #70, with the subsequent modifications as discussed in that thread, and a Changelog entry. It still lists you as the primary author of the patch, and I've added myself in a Co-authored-by tag. Does that work for you?

@fghaas fghaas requested a review from mrtmm February 20, 2024 16:54
The Open edX Quince release comes with a new django-storages version
(1.14).

New versions of django-storages have stricter checks for
STORAGE_KWARGS. It is no longer allowed to have options from other
storage classes, such as base_url, which is irrelevant to the S3
storage class.

The bucket option has also been removed. It has been named bucket_name
since django-storages 1.7 (even Open edX Lilac used 1.8, so there are
no backward compatibility concerns); 1.10 dropped the "bucket" alias.

Co-authored-by: Florian Haas <[email protected]>
@CefBoud
Copy link
Contributor

CefBoud commented Feb 20, 2024

Hey @fghaas
Thank you for taking care of that. LGTM ✅.
I will close the other PR.

@CefBoud CefBoud mentioned this pull request Feb 20, 2024
@fghaas fghaas merged commit 0d16f7e into hastexo:main Feb 21, 2024
4 checks passed
@fghaas fghaas deleted the fix-grade-storage-kwargs branch February 21, 2024 11:34
@fghaas
Copy link
Contributor Author

fghaas commented Feb 21, 2024

@CefBoud We've tagged v1.3.1, which you should be able to use in conjunction with Quince. I've tested profile images and grade report downloads on our staging platform, and all should be well.

@CefBoud
Copy link
Contributor

CefBoud commented Feb 21, 2024

@fghaas Thank you for the head's up. Much appreciated. 😄

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

Successfully merging this pull request may close these issues.

3 participants