search

openresty / lua-resty-limit-traffic

Lua library for limiting and controlling traffic in OpenResty/ngx_lua
819 stars 150 forks source link

bugfix: set_conn() didn't work #31

Closed pearzl closed 6 years ago

pearzl commented 6 years ago

hi,I found function set_conn() (which in conn.lua ) assign conn to self.conn which is never be used. And the test result is consistent with my guess : set_conn() do not make any effetc. It looks like a wrong name mistake, conn should change to max, if I didn't lost any thing important.

agentzh commented 6 years ago

@pearzl Good catch! Will you also add some test cases to cover this fix? This way we can make sure this won't be broken again in the future :) Thank!

pearzl commented 6 years ago

ok, I will. But I never done this before, i need some time to figure out how to do it.

agentzh commented 6 years ago

@pearzl Great. Thanks! BTW, you may find this tutorial helpful for writing tests:

https://openresty.gitbooks.io/programming-openresty/content/testing/

pearzl commented 6 years ago

@agentzh Sorry, I haven't got back to you sooner. I add a new test case for set_conn(), additionally, set_burst() is included in this case.

agentzh commented 6 years ago

@pearzl Many thanks!

@moonming Will you review this PR please?

pearzl commented 6 years ago

On Jan 14, 2018 09:25, Thibault Charbonnier notifications@github.com wrote:@thibaultcha commented on this pull request.

In t/conn.t:

@@ -235,3 +235,52 @@ committed: true --- no_error_log [error] [lua] + +

We are missing a line jump here. You should use the reindex script of the devel-utils repository.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

moonming commented 6 years ago

@agentzh it looks good to me. @pearzl thanks for your PR.

agentzh commented 6 years ago

@pearzl Merged with the following commit log message:

bugfix: set_conn() method did not work at all due to a copy&paste error.

Thanks!