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

Trying Limit.max_value being unsigned #124

Closed
wants to merge 3 commits into from
Closed

Conversation

alexsnaps
Copy link
Member

@alexsnaps alexsnaps commented Sep 6, 2022

This is by no mean complete or possibly even entirely correct, I brute-forced this in a bit… but I wanted to try figuring out why Limit.max_value is signed … and, well, I still don't know. Other than possibly integer arithmetic being "annoying" ?

@alexsnaps alexsnaps mentioned this pull request Sep 6, 2022
@alexsnaps
Copy link
Member Author

alexsnaps commented Sep 7, 2022

Alright, so Redis stores Integer as signed 64 bits and INCR is also on 64 bit signed integer only.

An error was signalled by the server: Error running script (call to f_32b544ae1f80cd9e6bdc62f5b477af978012c803): @user_script:3: ERR value is not an integer or out of range

because of this change

Having i64::MAX as an upper limit tho, doesn't invalidate using a u64 to store it… I don't know what's best tbh… #121 like approach or… this here… or having our own type?

@alexsnaps alexsnaps self-assigned this Sep 7, 2022
@alexsnaps alexsnaps added this to the Limitador v0.3 milestone Sep 7, 2022
@alexsnaps
Copy link
Member Author

That's the take away from this, we'll keep things as is in the crate for now and validate while parsing the file see #121

@alexsnaps alexsnaps closed this Sep 7, 2022
@alexsnaps alexsnaps deleted the unsigned_limit branch September 7, 2022 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant