janiussyafiq commented on PR #13651:
URL: https://github.com/apache/apisix/pull/13651#issuecomment-5447677752
Thanks for addressing my concerns, however few issues remain:
1. I ran the test locally and TEST 3 seems to fail. You can open up the PR
against your fork and ensure CI green.
2. `build_session_opts` now does core.table.clone per request, so the
`session.cookie.lifetime` is deprecated warning fires on every request for
users still on the alias (previously once, because absolute_timeout was written
back into the shared conf).
3. On the schema shape I still think an explicit session.revocation enum
({"redis"}) with revocation_fail_mode defaulting to open is the better
contract. Every other session.* key is a 1:1 forward to `lua-resty-session`.
This is the only derived one (by setting revocation field implicitly.
4. `--- no_error_log [crit]` on TEST 1 replaces the default [error] gate;
nothing in the flow logs [error], so it only hides regressions. Drop it, and
assert the replay is 302 rather than ~= 200.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
