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

[general] Write overwriting files in terms of .exists() #1422

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

jschneier
Copy link
Owner

@jschneier jschneier commented Jul 6, 2024

Follow upstream and write the file overwriting handling in terms of exists().

Reference: django/django@0b33a3a#diff-63fca472a0ca8c04505cb4ec9c132663ac71fc964110afc8ae9218416f37821cR195-R202

@jschneier jschneier merged commit a531947 into master Jul 6, 2024
19 checks passed
@jschneier jschneier deleted the josh/overwrite-via-exists branch July 6, 2024 21:51
@mikicz
Copy link

mikicz commented Jul 9, 2024

Btw found it very confusing that this change was not in the changelog!

@CorentinGoodays
Copy link
Contributor

CorentinGoodays commented Sep 5, 2024

I noticed the change where exists() now always returns False when allow_overwrite=True. I’m curious about the rationale behind this decision, as I often rely on the exists() method for troubleshooting storage issues to check whether a blob already exists.

With this new behavior, how would you recommend handling such cases when allow_overwrite=True? Is there an alternative approach you had in mind for checking the existence of files under this setting?

Thanks in advance for the clarification!

@jschneier
Copy link
Owner Author

jschneier commented Sep 11, 2024

Indeed, the intent is to walk back the behavior which is why no new version has been released. If someone wants to submit a patch that fixes this for all backends and doesn't introduce a security vulnerability (following Django's lead most likely) then I would happily merge.

github-merge-queue bot pushed a commit to mozilla/experimenter that referenced this pull request Oct 18, 2024
Reverts #11560

New version of django-storages introduced behavior that breaks
`exists()` when `allow_overwrite` is True. This is going to be reverted,
but there's no release with the revert yet. See this PR for more info:
jschneier/django-storages#1422

For now we can revert the library updates from yesterday (esp bc they
didn't fix the problem we were trying to fix at the time), and when
django-storages gets an update we can try again.
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.

4 participants