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

use late static binding for RedisExtension::fixClientConfig() #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkoubik
Copy link

@mkoubik mkoubik commented Sep 20, 2016

...so you can actually override it

@enumag
Copy link
Member

enumag commented Sep 20, 2016

👎 It seems hacky. Why do you need to override it? Maybe we can find a better solution for your use-case.

@mkoubik
Copy link
Author

mkoubik commented Mar 30, 2017

I've overriden loadNativeSessionHandler() method so it resolves host and port in runtime, not in compile time.
Original extension has $savePath = sprintf('tcp://%s:%d', $session['host'], $session['port']); - it resolves the save path once and hard-codes it into the container.
In my case, host and port parameters are being resolved dynamically in runtime, so $session['host'] is a DI\Statement and thus i have $savePath = new Statement('::sprintf', ['tcp://%s:%s', $host, $port]);.

@enumag
Copy link
Member

enumag commented Mar 30, 2017

In that case maybe it would be enough to change fixClientConfig to ignore Statement values?

@mkoubik
Copy link
Author

mkoubik commented Mar 30, 2017

Unfortunately not, because I cannot pass instance of Statement to sprintf() (and it would make no sense), I need to create a new Statement.

@enumag
Copy link
Member

enumag commented Mar 30, 2017

Well you'll still need to implement loadNativeSessionHandler on your own. But why do you need to override fixClientConfig as well? What would your implementation of fixClientConfig look like?

@mkoubik
Copy link
Author

mkoubik commented Apr 10, 2017

Because of $config['host'] instanceof Statement, original fixClientConfig() assumes $config['host'] is a string.

@enumag
Copy link
Member

enumag commented Apr 10, 2017

Would this work for you? enumag@fe6b9ac

@mkoubik
Copy link
Author

mkoubik commented Apr 13, 2017

Yes, it would be great.

@fprochazka fprochazka changed the title use late static binding for RedisExtension::fixClientConfig() use late static binding for RedisExtension::fixClientConfig() Apr 28, 2017
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