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

Remove redis-cache volume #1555

Merged
merged 3 commits into from
Jan 27, 2025
Merged

Conversation

sdfateh
Copy link
Contributor

@sdfateh sdfateh commented Jan 25, 2025

Details of the Change:

This pull request addresses an issue where the assets.json file, now stored in Redis by Frappe, persists across new image builds due to the Redis cache volume being persistent. This prevents updates to assets.json from being reflected after a new build.

The proposed change removes the persistent volume for redis-cache, ensuring that the cache, including assets.json, is cleared during each new image deployment.

Problem Solved:

By removing the persistent Redis volume, we prevent outdated assets.json data from being retained, ensuring the application always works with the latest assets after a build. This eliminates the need for manual intervention or additional automation to clear the cache.

Retaining the old assets.json file will cause the websites to miss certain assets:

image

Why is This Necessary?

Without this change, developers or pipelines must manually clear the assets.json value from Redis or automate the process using additional scripts or hooks. Removing the persistent volume simplifies the process and avoids potential misconfigurations.

This PR was inspired by @ankush's comment on the issue frappe/frappe#29901 , as he correctly pointed out that there is no need to persist in this volume.

@gparent
Copy link
Contributor

gparent commented Jan 26, 2025

I hope this comment isn't too spammy - I just wanted to thank you for trying to improve on this and also to the others who posted solutions. I think you'll save many people a lot of frustration.

Copy link
Member

@ankush ankush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks fine in principle.

I'd let someone with better knowledge of frappe_docker merge it though.

@revant revant merged commit bcb2035 into frappe:main Jan 27, 2025
9 checks passed
revant added a commit to kelvinelove/frappe_docker that referenced this pull request Jan 27, 2025
* Remove redis-cache volume

* remove from docs

* ci: fix pre-commit

---------

Co-authored-by: Revant Nandgaonkar <[email protected]>
revant added a commit that referenced this pull request Jan 27, 2025
* Update custom-apps.md

grammar fixes

* Remove redis-cache volume (#1555)

* Remove redis-cache volume

* remove from docs

* ci: fix pre-commit

---------

Co-authored-by: Revant Nandgaonkar <[email protected]>

---------

Co-authored-by: Salah Aldin Fateh <[email protected]>
Co-authored-by: Revant Nandgaonkar <[email protected]>
@Ph0en1XXX
Copy link

Ph0en1XXX commented Jan 27, 2025

Hi! After removing volumes and docker-compose up -d I got a new "404 Not Found". Could deleting the Redis cache volume have affected this error?
image
image

@sdfateh sdfateh deleted the Remove-redis-cache-volume branch January 27, 2025 07:22
@sdfateh
Copy link
Contributor Author

sdfateh commented Jan 27, 2025

@Ph0en1XXX

How did you handle volume removal? It seems like you deleted the sites volume

@Ph0en1XXX
Copy link

Ph0en1XXX commented Jan 27, 2025

I did docker-compose down -v
When reinstalledfull images using the script for lms, but with my own image.

python3 ./easy-install.py deploy
--project=learning_prod_setup
--email=your_email.example.com
--image=ghcr.io/frappe/lms
--version=stable
--app=lms
--sitename subdomain.domain.tld

@Ph0en1XXX
Copy link

Ph0en1XXX commented Jan 28, 2025

I did docker-compose down -v When reinstalledfull images using the script for lms, but with my own image.

python3 ./easy-install.py deploy --project=learning_prod_setup --email=your_email.example.com --image=ghcr.io/frappe/lms --version=stable --app=lms --sitename subdomain.domain.tld

Fixed with just launching default script image then changed to my own (learning_prod_setup-compose.yml)

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.

6 participants