Skip to content

Fix CacheSequenceResolver for Redis with Compression & Serialization #13

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

Closed
wants to merge 5 commits into from

Conversation

rzv-me
Copy link
Contributor

@rzv-me rzv-me commented Feb 28, 2025

Code is now compatible with Compression and Serialization enabled for Redis.

The implementation is the same as Laravel used for RateLimiting using Redis.

src/Illuminate/Cache/RateLimiter.php

Without this change, I get this error when trying to create a snowflake ID:

InvalidArgumentException 

Sequences must be an integer between 0 and 4095 (got -1).

…tion on Redis

Code is now compatible with Compression and Serialization for redis. 

The implementation is the same as Laravel used for RateLimiting using Redis.

https://github.com/laravel/framework/blob/5d477f9a4080c1cdd43edd05ec4df61c738e0f8d/src/Illuminate/Cache/RateLimiter.php#L161
@rzv-me rzv-me changed the title Fix CacheSequenceResolver for Redis Compression & Serialization Fix CacheSequenceResolver for Redis with Compression & Serialization Feb 28, 2025
@rzv-me
Copy link
Contributor Author

rzv-me commented Mar 3, 2025

@inxilpro any ETA on approving and merging this?

@inxilpro inxilpro closed this Mar 4, 2025
@rzv-me
Copy link
Contributor Author

rzv-me commented Mar 4, 2025

@inxilpro FYI the method doesn't exist prior to laravel 11, I am pushing a change to fix it

@inxilpro
Copy link
Contributor

inxilpro commented Mar 4, 2025

Hm, it shows that I closed this but I didn't mean to. Did you?

@rzv-me
Copy link
Contributor Author

rzv-me commented Mar 4, 2025

no, I think some workflow triggered it and it also tagged a new version

@inxilpro
Copy link
Contributor

inxilpro commented Mar 4, 2025

OK. Ping me when it's ready to look at again.

@rzv-me
Copy link
Contributor Author

rzv-me commented Mar 4, 2025

done, had to recreate the logic as the method is not available before Laravel 11.x.
Tested with both 10.x and 11.x
Opened a different PR #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants