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

Support for Heroku style REDIS_URL env variables #353

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

iKlaus
Copy link
Contributor

@iKlaus iKlaus commented Jun 9, 2017

Being affected by the issue described in #302 I looked into what needs to be done to support Heroku's REDIS_URL environment variables. In short, they provide the credentials to a booked Redis addon via an environment variable which might change anytime at their discretion:

In order for Heroku to manage this add-on for you and respond to a variety of operational situations, the REDIS config vars may change at any time. Relying on the config var outside of your Heroku app may result in you having to re-copy the value if it changes.
-- https://devcenter.heroku.com/articles/heroku-redis#create-a-new-instance

Therefor I just wanted to be able to make use of the env() dynamic parameter notation as in

snc_redis:
    session:
        client: session
    clients:
        session:
            type:  predis
            alias: session
            dsn:  "%env(REDIS_URL)%"

I came up with a rather simple approach which basically boils down to:

  • detect the specific syntax while the container compiles and bypass the validation on compilation
  • assemble the service with the help of a Factory which parsed the url at runtime
  • merge parsed url fragments with the rest of the connection options and then instantiates the Predis client as usual

@iKlaus iKlaus force-pushed the heroku-redis-url branch 2 times, most recently from 75da211 to 29290d9 Compare June 12, 2017 15:02
@iKlaus iKlaus force-pushed the heroku-redis-url branch from 29290d9 to c3f82c5 Compare June 12, 2017 15:15
@iKlaus
Copy link
Contributor Author

iKlaus commented Jun 12, 2017

So, fixed the tests and the code to comply with the projects extreme BC support :)

@pamil
Copy link

pamil commented Jul 11, 2017

Just a friendly ping :)

@snc snc merged commit 25a1aa0 into snc:master Jul 12, 2017
@snc
Copy link
Owner

snc commented Jul 12, 2017

Merged this great PR, thanks for your work @iKlaus!

@nicholasruunu
Copy link

@snc Did you forget to tag this merge?

@snc
Copy link
Owner

snc commented Jul 18, 2017

@nicholasruunu No, I silently hope for some more feedback from successful uses ;-)

@nicholasruunu
Copy link

@snc We're currently using this in two apps now, although both are still on stage. Both using redis for sessions and nothing else.

@aurimasniekis
Copy link
Contributor

This does not support phpredis

PHP Fatal error:  Uncaught Symfony\Component\Debug\Exception\UndefinedMethodException: Attempted to call an undefined method named "getSocket" of class "Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn". in /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php:277
Stack trace:
#0 /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php(108): Snc\RedisBundle\DependencyInjection\SncRedisExtension->loadPhpredisClient(Array, Object(Symfony\Component\DependencyInjection\ContainerBuilder))
#1 /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php(52): Snc\RedisBundle\DependencyInjection\SncRedisExtension->loadClient(Array, Object(Symfony\Component\DependencyInjection\ContainerBuilder))
#2 /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/symfony/dependency-injection/Compiler/MergeExtensionConfigurationPass.php(59): Snc\RedisBundle\DependencyInjection in /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php on line 277

Fatal error: Uncaught Symfony\Component\Debug\Exception\UndefinedMethodException: Attempted to call an undefined method named "getSocket" of class "Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn". in /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php on line 277

Symfony\Component\Debug\Exception\UndefinedMethodException: Attempted to call an undefined method named "getSocket" of class "Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn". in /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/snc/redis-bundle/DependencyInjection/SncRedisExtension.php on line 277

Call Stack:
    0.3484   12233472   1. Symfony\Component\Debug\ErrorHandler->handleException() /Users/aniekis/Projects/Work/Web/PHP/core-api/vendor/symfony/debug/ErrorHandler.php:0

There is half of getters missing on RedisEnvDsn compared to RedisDsn...

@snc
Copy link
Owner

snc commented Jan 16, 2018

#378 includes related changes, it would be nice if some of you can test their setups.

@aurimasniekis
Copy link
Contributor

Sorry, but I am not using this anymore as it was causing problems back in the days...

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.

5 participants