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

Allow setting a custom backend storage #4

Merged
merged 5 commits into from
May 7, 2018

Conversation

esolitos
Copy link

With this change the secondary storage bin can be customised on demand, allowing to define, for example, another memory storage.
This is can be particularly useful in conjunction with wodby/drupal-varnish#3 because it allows the bin to be completely customised.

Note This is an extension of VARNISHD_STORAGE_SIZE, making it more customisable while preserving backward compatibility.

…g backward compatiblity with VARNISHD_STORAGE_SIZE
@csandanov
Copy link
Member

I think it would be better to use something like VARNISHD_SECONDARY_STORAGE where you can specify storage name, type, path and size.

@esolitos
Copy link
Author

esolitos commented May 2, 2018

I agree, that VARNISHD_SECONDARY_STORAGE is a better name for the variable (changed it now), however I was thinking to leave the storage name fixed to secondary, just to prevent the need for a second variable for the storage name in the use-case: wodby/drupal-varnish#3
However if you think it's better leaving the storage name configurable as well, I'll change that as well.

@csandanov
Copy link
Member

What I meant is to have just a single env var VARNISHD_SECONDARY_STORAGE instead of two env vars. As the example value it could be:

VARNISHD_SECONDARY_STORAGE=file,/var/lib/varnish/storage.bin,10MiB

Because now variable VARNISHD_STORAGE_SIZE actually means "add additionall storage with type file, named secondary, with the path /var/lib/varnish/storage.bin and with the size what have specified". We can keep name secondary because it will be used in the variable name.

@esolitos
Copy link
Author

esolitos commented May 3, 2018

Ok, the idea was to leave VARNISHD_STORAGE_SIZE for backward compatibility, do you think I should simply drop it?

Last commits are just small changes: renamed the variable and changed the priority since the VARNISHD_STORAGE_SIZE would be deprecated.

@csandanov
Copy link
Member

Ohh, I'm sorry, I didn't realize we had it before :) yes, I think we should drop it, it's stupid

@esolitos
Copy link
Author

esolitos commented May 7, 2018

Updated dropping VARNISHD_STORAGE_SIZE in favour of the more generic VARNISHD_SECONDARY_STORAGE.

@csandanov csandanov merged commit 783ae76 into wodby:master May 7, 2018
@esolitos esolitos deleted the feature/alternative-storage-option branch May 16, 2018 10:07
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.

2 participants