set_rotate #8

Merged
merged 2 commits into from Apr 17, 2012

Conversation

Projects
None yet
2 participants

viperet commented Mar 30, 2012

Hi! I've a new dirrective set_rotate $value $from $to, which "rotates" $value in range $from - $to
Sample config:

   location /rotate {
       default_type text/plain;
       set $counter $cookie_counter;
       set_rotate $counter 1 5;
       echo $counter;
       add_header Set-Cookie counter=$counter;
   }                                                                                                      

Accessing "/rotate" will display numbers from 1 to 5 in cycle. I use this directive for rotating banners.
I've added some basic documentation to doc/HttpSetMiscModule.wiki

Owner

agentzh commented Mar 31, 2012

This looks very interesting :) I'll merge your patches later when I have some time ;) Thank you for your contribution!

Owner

agentzh commented Apr 17, 2012

Sorry for the delay! It seems to me that another interesting feature here is when $counter is uninitialized, i.e., not found or just empty strings, then we could use a persistent state in the nginx worker process level, such that we can do round-robin dispatch for upstreams.

Also, will you submit some test cases for this new feature?

Thank you!

Owner

agentzh commented Apr 17, 2012

No worries, I'm already working on these :)

Owner

agentzh commented Apr 17, 2012

I've fixed various coding style issues and typos in your patches:

https://github.com/agentzh/set-misc-nginx-module/commit/9e061396

I've also added some test cases for this new directive.

I'll then work on the persistent rotating state for this directive when there is no "current value" given.

Owner

agentzh commented Apr 17, 2012

I've already made the "current value" persistent and set_rotate will use it when the current value is not given or is given a bad value. See the patch here:

https://github.com/agentzh/set-misc-nginx-module/commit/9792e8d

Does it look good for you?

Owner

agentzh commented Apr 17, 2012

The "current value" persistence is also on a per-location basis, see the following (passing) test case:

1c8e3ef

viperet commented Apr 17, 2012

Thanks for correcting and fixing my code.

Does it look good for you?

It's very good to me!

agentzh merged commit f091d5b into openresty:master Apr 17, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment