Skip to content

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