search

YunoHost-Apps / Experimental_helpers

6 stars 12 forks source link

Redis helper #20

Closed Jibec closed 6 years ago

Jibec commented 6 years ago

FYI, I started a Redis helper, I'm unsure this is useful to share together for now, please write here your thoughts about it.

I had to make sure two Funkwhale installation are not using the same redis database

https://github.com/YunoHost-Apps/funkwhale_ynh/pull/4 (the helper + usage

I assume if I remove the database and another app uses it, it will remove it's cache, how bad can it be?

Here is an example with discourse: https://github.com/discourse/discourse/blob/master/config/discourse_defaults.conf#L115 https://github.com/YunoHost-Apps/discourse_ynh/blob/master/scripts/install#L130

JimboJoe commented 6 years ago

I think this a good catch!! :+1: When reading about it on Discourse forum, there have been some redis database "collisions" where Discourse just flushed the database 0, and some other apps were using it... So this helper is really an excellent idea! I read your helper, and the principle seems good, 2 small remarks:

As for the database deletion, I agree with you that provided apps use this helper, we can safely remove the database (and existing apps don't specify database, hence using database 0 which will never get removed).

But I'll definitely try to implement your helper in Discourse! And I didn't find a redis database parameter for wallabag2, but there's one for Nextcloud...

Jibec commented 6 years ago

Thanks for your feedback, here is an improvement:

https://github.com/YunoHost-Apps/funkwhale_ynh/commit/9615629ed20bbbaf821152c374c76ec808c01300#diff-dc2801cb46a5d60e62234cff98084498

I don't know how to handle a situation where redis doesn't have any more database available, so I decided to trigger an error.

What to you think?

Jibec commented 6 years ago

Here is the pull request: https://github.com/YunoHost-Apps/Experimental_helpers/pull/23 (please continue the conversation on the PR)