fix(src): validation log with notice level #49
Conversation
chronolaw
force-pushed
the
fix/change_log_to_notice
branch
from
233a3ef
to
7d49fc8
Compare
src/ngx_lua_resty_lmdb_module.c
Outdated
| @@ -539,7 +539,7 @@ static ngx_int_t ngx_lua_resty_lmdb_init(ngx_cycle_t *cycle) | |||
| if (ngx_lua_resty_lmdb_validate(cycle, lcf) != NGX_OK) { | |||
| ngx_lua_resty_lmdb_close_file(cycle, lcf); | |||
|
|
|||
| ngx_log_error(NGX_LOG_WARN, cycle->log, 0, | |||
| ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, | |||
There was a problem hiding this comment.
I want this to be more visible as wiping the database is a destructive operation. Why would this message occur in our tests anyway? We are not testing the upgrade in integration tests.
Also, could you test by changing the .requirments file to point to this branch and shows the test passes? I want to make sure this PR is good before merging.
There was a problem hiding this comment.
When kong first start, there is no tag in lmdb, and we have set validation tag in nginx-kong.conf, so the C module will wipe the lmdb database and leave a log message.
If the log is warning level, it will break the test case in spec/02-integration/02-cmd/02-start_stop_spec.lua.
assert.logfile().has.no.line("[warn]", true)
There was a problem hiding this comment.
PR Kong/kong#12026 is using this branch.
There was a problem hiding this comment.
Perhaps we could add another condition:
if the lmdb database is empty then write the tag into database and don't think it is an warning.
There was a problem hiding this comment.
if the lmdb database is empty then write the tag into database and don't think it is an warning.
THis should not be necessary, for our use case we can assume lack of tag = empty database. We do not concern compatibility with other LMDB using applications.
#34 introduced a new directive
lmdb_validation_tag, but the mismatch log levelWARNis too high,this PR change the log level to
NOTICE.This may fix the ci test: https://github.com/Kong/kong/actions/runs/6876582885/job/18702488766
Related PR: Kong/kong#12026