Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(db): add back FLOOR so ttl returns integer instead of float #9960

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

outsinre
Copy link
Contributor

@outsinre outsinre commented Dec 14, 2022

Summary

Add back FLOOR so that ttl is returned as integer instead of float.

PR #9801 remove FLOOR part to reflect the real ttl value, but may return a float value instead of int, which would break some downstream clients as depicted in issue #9944.

This PR add back FLOOR as in the handler.lua, we have double-check on cred.ttl == 0. So it is safe to add back.

Also check FTI-4066.

Issue reference

Fix #[9944]

@chronolaw
Copy link
Contributor

Could we add a test case to catch this bug?

@outsinre
Copy link
Contributor Author

Could we add a test case to catch this bug?

We just add back a removed part, which has built-in test already.

But also need verification from downstream clients that relies on int return, like [9944].

@chronolaw
Copy link
Contributor

We should add a change log entry too.

@chronolaw chronolaw linked an issue Dec 14, 2022 that may be closed by this pull request
1 task
@outsinre outsinre changed the title fix(db) make sure ttl returns integer instead of float fix(db) add back FLOOR so ttl returns integer instead of float Dec 14, 2022
@chronolaw chronolaw changed the title fix(db) add back FLOOR so ttl returns integer instead of float fix(db): add back FLOOR so ttl returns integer instead of float Dec 14, 2022
@outsinre outsinre force-pushed the FTI-4066-make-sure-integer-return branch from 2ed8e4f to f57af06 Compare December 14, 2022 08:18
@outsinre
Copy link
Contributor Author

We should add a change log entry too.

Done.

Add back `FLOOR` so that `ttl` is returned as integer instead of float.

PR <#9801> remove `FLOOR` part to reflect the real `ttl` value, but may return a `float` value instead of `int`, which would break some downstream clients as depicted in issue <#9944>.

This PR add back `FLOOR` as in the `handler.lua`, we have double-check on `cred.ttl == 0`. So it is **safe** to add back.

Fix #_[9944]_ and also check [FTI-4066].
@outsinre outsinre force-pushed the FTI-4066-make-sure-integer-return branch from f57af06 to 791488e Compare December 14, 2022 09:07
@outsinre outsinre requested a review from dndx December 14, 2022 09:07
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Datong Sun <[email protected]>
@dndx dndx merged commit 99cdbea into master Dec 14, 2022
@dndx dndx deleted the FTI-4066-make-sure-integer-return branch December 14, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin API returns float for key-auth TTL
3 participants