westerveltco / django-twc-project

A Django template for all web applications at The Westervelt Company
MIT License
25 stars 3 forks source link

add CACHES to settings, defaulting to locmem #319

Closed joshuadavidthomas closed 2 months ago

joshuadavidthomas commented 2 months ago

@jefftriplett Re: the dummycache, that's already in place https://github.com/westerveltco/django-twc-project/blob/59edc06dd2049b656937c99223662a29367fdb7d/src/django_twc_project/tests/settings.py.jinja#L9-L13

What about checking for the presence of the CACHE_URL env var, and if it's not present falling back to the in memory?

joshuadavidthomas commented 2 months ago

Ah, you can set the localmem via the cache url https://github.com/epicserve/django-cache-url#supported-caches

locmem (default): 'locmem://[NAME]'

I'm not opposed to that, but like I mentioned in one of our most recent meetings my only worry would be memory usage and how our applications are currently deployed. I've never done any benchmarking around this, so I do not know how much memory the caching would take and whether it would make a measurable difference where we would have to bump the specs up on the app machines. If you have some experience in this area, I'm all ears.

joshuadavidthomas commented 2 months ago

On the other hand, I probably run those machines a little too close to the edge in terms of allocated memory, so maybe they could all stand to be bumped a bit. 🤷

jefftriplett commented 2 months ago

I'm not opposed to that, but like I mentioned in one of our most recent meetings my only worry would be memory usage and how our applications are currently deployed.

Yes, but this is the default, and we aren't seeing any apps run out of memory. I think the change is fine, but there might be a non-zero impact if we were to set it to one of fly's persistent volumes. (which we aren't using)

joshuadavidthomas commented 2 months ago

Ok, you've convinced me. I'm probably being a bit overly worried about it anyway.