Skip to content

Conversation

@ribeiroit
Copy link

I've added a new function to concatenate the arrays values as a string separated by the wildcard ":".

A new file example file "arrays.vcl" was generated to explain the correct usage. The great motivation to develop this new resource is reduce the number of connections that varnish runs on redis and makes everything with just one query.

…ated by the wildcard ":". A new file example file "arrays.vcl" was generated to explain the correct usage. The great motivation to develop this new resource is reduce the number of connections that varnish runs on redis and makes everything with just one query.
@andreacampi
Copy link
Member

Fixing the conversion of array values is a great thing, thanks for taking care of that.
I'd love the merge this but there are a few things that IMHO should be improved; I've added a few comments inline, please review them.

That said, I'm not sure I understand the example; or rather, why do you need a hash in this particular case.
Wouldn't the following accomplish the same:

redis-cli> set vrnsh:mydomain.com 'r:anotherdomain.com:301'
redis-cli> set vrnsh:mydomaintwo.com 'd:be2'

set req.http.redis_key = redis.call("GET vrnsh:" + req.http.host);

I'm not disagreeing, just wondering—but I do agree we need to support arrays as replies.

@ribeiroit
Copy link
Author

Cause I have a provisioning system that uses redis hashes, with another more objects, and when I tried to use your lib it didn't support arrays. For me it was a problem to send many requests to redis reading element by element of the hash, so I decided to return an array as string and use regex inside the vcl. Using hash I can use commands like sismember and more in my provisioning system.

You pointed me a good example without hash, but I already had everything made using hashes and it would be a big headache to change that. Thank you about your answer and think that it'll be useful for more users that uses hashes.

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