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

Add phpredis serialization and compression config support #36791

Closed
wants to merge 1 commit into from

Conversation

TheLevti
Copy link
Contributor

This pull request allows the framework user to configure phpredis (https://github.com/phpredis/phpredis) serialization and compression options right in the config instead of the need to overwrite the service provider or a custom driver.

Supported serializers:

  • NONE
  • PHP
  • JSON
  • IGBINARY
  • MSGPACK

Supported compressors:

I did here similar changes to what #31262 tried to do, just without the for me not yet understandable other adjustments to some of the redis functions. Also added tests for different compression algorithms.

Note: My first attempt was to always serialize + compress the passed arguments to eval, but then later decided to let the user take care of this as we can not know what kind of lua script he wants to execute and whether all arguments are expected to be serialized/compressed. Instead I provided a helper method on the PhpredisConnection class that the user may use. In addition I refactored the previous phpredis lock support (#36337) for these functionalities to use this new helper method.

]))->connection();
}

// TODO: Uncomment after https://github.com/phpredis/phpredis/issues/1939
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking as draft, due to this blocker.

@driesvints
Copy link
Member

Please send this in when the PR is finished and without TODO's. Thanks

@driesvints driesvints closed this Mar 29, 2021
@TheLevti
Copy link
Contributor Author

Thanks, will first try to fix phpredis, if thats not possible short term, I will exclude lz4 support.

@it-can
Copy link
Contributor

it-can commented Oct 25, 2021

@TheLevti any news on this?

@TheLevti
Copy link
Contributor Author

TheLevti commented Nov 1, 2021

@TheLevti any news on this?

I am waiting for phpredis to release 5.3.5

@it-can
Copy link
Contributor

it-can commented Jan 2, 2022

@TheLevti any news on this?

I am waiting for phpredis to release 5.3.5

@TheLevti 5.3.5 is released I see https://github.com/phpredis/phpredis/releases/tag/5.3.5

@TheLevti
Copy link
Contributor Author

TheLevti commented Jan 5, 2022

@TheLevti any news on this?

I am waiting for phpredis to release 5.3.5

@TheLevti 5.3.5 is released I see https://github.com/phpredis/phpredis/releases/tag/5.3.5

Thanks for the update, will re-open my pull request soon.

@TheLevti
Copy link
Contributor Author

TheLevti commented Jan 6, 2022

#40282 follow up

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.

4 participants