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

Docker will now force-remove rendition entries from db #216

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Eeemil
Copy link

@Eeemil Eeemil commented Jul 26, 2018

Since Docker will use a new file system on a rebuild, old renditions in
$MEDIA/images will no longer be present. By removing rendition entries from db
Wagtail will be forced to generate new renditions, instead of providing dead
links to the old renditions.

Since Docker will use a new file system on a rebuild, old renditions in
$MEDIA/images will no longer be present. By removing rendition entries from db
Wagtail will be forced to generate new renditions, instead of providing dead
links to the old renditions.
@cnk
Copy link
Member

cnk commented Sep 8, 2020

@Eeemil Sorry this PR did not get reviewed sooner. The management command to clear renditions from the database is really helpful.

I am not 100% sure about running it from docker-entrypoint.sh. It looks to me like the Dockerfile is trying to be an example of how one might build production images (starting with the slim image and clearing up caches as you go). If I interpreted this correctly, then I am concerned about code that always clears the image renditions when the container starts (even though it is just fine for the demo).

Can you help me think through when this command should run - and when it will run? When I stopped my containers using Ctrl-C to stop docker-compose up, the database and the image renditions were both preserved. docker-compose up -d followed by docker-compose stop also preserved the database and rendition files. Using docker-compose down got rid of the database (and presumably the files).

I think the need for your delete_renditions command comes when the database data is preserved by the app container is not. I think that combination does not occur when using the instructions and Dockerfile + docker-compose.yml in this project. It happens when using the Dockerfile and docker-compose.yml in https://github.com/wagtail/docker-wagtail-develop (which does preserve the database volume). In which case we need to add the management command here and then use it from https://github.com/wagtail/docker-wagtail-develop.

@Amondale
Copy link
Contributor

Amondale commented Sep 12, 2021

@Eeemil you're right:
The management command to clear renditions from the database is really helpful.

My thinking is that Dockerizing any file-based app or set of apps is a "shifting sands" proposition, and rather than welding in a command to clear what may or may not be a bunch of orphan renditions, why not just ship it from github and document it along with the use cases that make it a valuable utility. "load_initial_data" and "delete_renditions" are needed in the early dev ("hack") phases of experimenting with the bakerydemo/Wagtail learning experience.

Is there any reason that delete_renditions can't be run at any point, even in a prod setting? It will temporarily reduce performance, but eliminate the mysterious "Not found" http errors that never show the particular rendition in a "not-found" state, nor state that it's an issue in the wagtailimages_rendition table.

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