thelounge / thelounge-docker

🐳 ‎ ‎Docker image for The Lounge, a self-hosted web IRC client
https://ghcr.io/thelounge/thelounge
MIT License
306 stars 73 forks source link

feat: add sqlite to thelounge container #179

Closed buroa closed 11 months ago

buroa commented 11 months ago

Adding sqlite to thelounge container to make it easier to prune logs

https://github.com/thelounge/thelounge/wiki/Purging-logs-older-than-X-days

onedr0p commented 11 months ago

This would be nice because you can run the prune command using the same container image.

brunnre8 commented 11 months ago

Don't think that's a good idea. Directly messing with the DB is unsupported. We should not encourage this in any way and hence the container doesn't need sqlite

buroa commented 11 months ago

Don't think that's a good idea. Directly messing with the DB is unsupported. We should not encourage this in any way and hence the container doesn't need sqlite

Why have you have it documented in your wiki then? Don't contradict yourself.

brunnre8 commented 11 months ago

it's a wiki, can be edited by anyone and in fact is. I'm not contradicting myself in amy way

brunnre8 commented 11 months ago

If you are messing with the DB, TL needs to be stopped anyway. It expects to be the sole writer. So if you run it in a container, you are supposed to stop the container first, so having sqlite in there seems pointless, just spin up a container that only contains that and mount the db volume into it.

Docker containers are supposed to contain the bare minimum to keep the app running, not development tools.

buroa commented 11 months ago

it's a wiki, can be edited by anyone and in fact is. I'm not contradicting myself in amy way

You reviewed it: https://github.com/thelounge/thelounge/wiki/Purging-logs-older-than-X-days/_history

If you are messing with the DB, TL needs to be stopped anyway. It expects to be the sole writer. So if you run it in a container, you are supposed to stop the container first, so having sqlite in there seems pointless, just spin up a container that only contains that and mount the db volume into it.

Docker containers are supposed to contain the bare minimum to keep the app running, not development tools.

We know it needs to be stopped. I created a brand new sqlite container to get around this limitation, but it would be nicer if it was build into this upstream version. In K8s, we typically utilize init containers which are created and completed before starting the main containers, in this case TL.

Refs: https://github.com/onedr0p/containers/pull/897 https://github.com/buroa/k8s-gitops/commit/bb2335eccfef17917b71faa7089eb649828783a7

brunnre8 commented 11 months ago

You reviewed it:

I added the you are on your own disclaimer, yes. Considering that we don't have a built-in way of doing it I didn't think that deleting it completely was the right course of action 😉

Doesn't mean that this suddenly counts as officially supported

onedr0p commented 11 months ago

Docker containers are supposed to contain the bare minimum to keep the app running, not development tools.

I agree, however since there is a major issue in the growth of the database over time some sacrifices could be made.

For example you could stop the running container, then run the same container with the sqlite commands to clean up the database. Then start the same container to run the app. This is beneficial in that you don't need another image to handle it or install sqlite on the machine running the app.