feature: implemented shdict:ttl() and shdict:expire() APIs #140
Conversation
|
As soon as I hear this API is a go, I'll provide documentation for it! |
|
@thibaultcha Sorry for the delay on my side. I'll have a look later today. Thanks! |
|
@thibaultcha What is the sister PR in lua-nginx-module? |
|
@thibaultcha Never mind, just found the sister ngx_lua PR myself: openresty/lua-nginx-module#1150 |
|
Ok :) (It's in the list of referenced PRs in the GitHub UI, but since I referenced this PR in several other places, it became harder to distinguish. Sorry!) |
|
@thibaultcha No worries :) |
lib/resty/core/shdict.lua
Outdated
| if key_len == 0 then | ||
| return nil, "empty key" | ||
| end | ||
| if key_len > 65535 then |
There was a problem hiding this comment.
Better add a blank line between successive if statement blocks.
There was a problem hiding this comment.
I certainly concur. It was only like so to be consistent with the above incr() method. Fixed.
lib/resty/core/shdict.lua
Outdated
| if key_len == 0 then | ||
| return nil, "empty key" | ||
| end | ||
| if key_len > 65535 then |
thibaultcha
force-pushed
the
feat/shm-ttl-expire
branch
from
8719dc4 to
cde7124
Compare
|
It seems like a recent release of Luacheck might have (once again) broken its default settings and now reports errors for this codebase? |
|
@thibaultcha Should we make the luacheck warnings non-fatal in travis testing? |
|
@agentzh I am not sure if you are asking for my opinion, but if so, then I am a big fan of enforcing the linter check in CI. However, I only dedicate a single Travis job to linting, like here: https://travis-ci.org/thibaultcha/lua-resty-mlcache This way, the CI test suites of a project is not impacted by potential linter failures (tests can still be successful even if the linter is complaining about something). It is a better separation of concerns in the CI imho. Happy to add those CI changes in #143 if you think they'd be valuable for lua-resty-core. |
|
@thibaultcha I'm not sure if that would be a good use of your valuable time :) Maybe we can just stick with a particular version of luacheck to avoid such regressions? |
Definitely a solution I have used in the past as well 👍 (I can try to keep an eye out for Luacheck releases to avoid lagging too far behind) |
|
@thibaultcha Cool, thanks! |
|
@dndx Thank you for your review! |
|
@thibaultcha I'd propose the following patch to make the test better: diff --git a/t/shared.t b/t/shared.t
index a4041b5..41d3bf8 100644
--- a/t/shared.t
+++ b/t/shared.t
@@ -129,10 +129,11 @@ failed to get ttl: not found
}
--- request
GET /t
---- response_body_like
-0.2
+--- response_body_like chomp
+\A0.2
sleep for 0.1s...
-0.0\d*
+-?0.0\d*
+\z
--- no_error_log
[error]
[alert]What do you think? |
|
Merged. Thanks! Will you create another PR to lua-nginx-module to document these new API functions? Thanks! |
|
Great! Good idea for the test. Yes, I'll propose those docs as soon as I can. Do you have a prefered timeline? Some deadline? ASAP maybe? |
|
@thibaultcha This week maybe? Next week is fine too :) Not that urgent. Thanks! |
|
Sounds good! I'll try to allocate some time to it. |
|
@thibaultcha Great! Thanks! |
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-resty-core project.
Hi there,
This is a stab at:
No documentation for now, just a proposal. I thought about including this API in a
ngx.sharedmodule the user would have to import explicitly (since they don't have Lua_CFunction counterparts), but went the easy way for now.Probably needs a bit more polishing, open to feedback here!
I would have opened a PR for
shdict:get_info()as well but apparently have been beaten to it :)