-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Remove files from cloud storage when version is wiped. #6186
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
Remove files from cloud storage when version is wiped. #6186
Conversation
Should we add a FAQ question for this? Edit: Added the FAQ. |
Not sure about that, if the files aren't refreshed that seems like a bug from our side. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I agree with @stsewd's suggestions, however, that any renaming of functions should probably be in a separate PR if at all. Let's make this PR just about removing files from media storage.
----------------------------------------------- | ||
|
||
If the search results contains stale/outdated results, | ||
you can update the search index by :doc:`wiping the build environment <guides/wipe-environment>` and then rebuilding your docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this. I mean, if that is happening I would like that users report that to us, since it is a bug from our side. We could remove this or add something like if this is happening very often, report it.
@davidfischer what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stsewd
I think this is a bug from Sphinx, if a .rst
file is deleted, then the .html
file for that shouldn't be created or deleted in the next build.
I think it would be better if a user tries wiping the environment and rebuilding the docs, if that doesn't work -- then report to us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, as we move toward each build being totally fresh and not relying on a previous environment, this problem should naturally go away. It is possible that we need to detect when files aren't generated as part of the build and remove them. Even with this change, that won't be done exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is not correct. Why? Because it changes the meaning of a Wipe.
Before this PR, the Wipe action only takes action on the builders (deleting some cached files). After this PR, if I understand correctly, the Wipe will also remove the live documentation for that version.
@humitos : what do you think should be the way to trigger the removal from cloud storage? As someone following this issue from outside, the primary issue for me is that I have a bad search index in cloud storage and no way at all to fix it. |
I'm not proposing a solution for the issue. I'm not sure how it should be. I'm saying that the proposed solution in this PR could introduce a behavior (a bug, to me) that we don't want. I may be wrong, though. |
@humitos I think you may be right. Then I think the problem is something similar to what we had in .com with the indexes. We are reading the json objects from the wrong directory. |
And also, maybe we are not cleaning the old files from storage before a new build as we do with the file based storage |
I think that @davidfischer can provide more info here. |
Credit to @stsewd but I think the issue is that with filesystem backed storage, we were running rsync which both copied new/updated files and deleted removed files. The current storage system doesn't do that with cloud storage. |
Fixes #6069