Skip to content
This repository was archived by the owner on Aug 26, 2021. It is now read-only.

Add support for Heroku Redis #31

Merged
merged 4 commits into from
Mar 13, 2016
Merged

Add support for Heroku Redis #31

merged 4 commits into from
Mar 13, 2016

Conversation

mattstibbs
Copy link
Contributor

Added support for Heroku Redis which sets an environment variable of REDIS_URL.

Not sure if I've missed some implication of simply adding like this but worth a pull request!

@singingwolfboy
Copy link
Contributor

This looks good to me! Can you actually put the Heroku Redis section before Redis To Go? I think it makes more sense that way.

@mattstibbs
Copy link
Contributor Author

@singingwolfboy Thanks - have swapped those sections around now.

Is it ok that for someone using both Heroku Redis and Redis To Go, Redis To Go would take precedence? Not that I can think of a different way of handling someone using both add-ons at the same time :)

@singingwolfboy
Copy link
Contributor

Actually, because you're using .setdefault(), the first section will take preference, not the last. That's one of the reasons I suggested you switch them. 😄

app.config.setdefault('REDIS_HOST', url.hostname)
app.config.setdefault('REDIS_PORT', url.port)
app.config.setdefault('REDIS_PASSWORD', url.password)

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove trailing whitespace from this line.

@mattstibbs
Copy link
Contributor Author

@singingwolfboy Thanks - whitespace removed (and I'll watch out for that in future)

@singingwolfboy
Copy link
Contributor

@mattstibbs You missed a line :)

@mattstibbs
Copy link
Contributor Author

@singingwolfboy Ah damn. Think I've got it now? (thanks for being patient!)

@singingwolfboy
Copy link
Contributor

Looks great! I'll merge this now. I'll wait a few days to see if @brainix is going to submit another PR (as he said he would in #27). Then, I'll cut a new release and push it to PyPI.

singingwolfboy added a commit that referenced this pull request Mar 13, 2016
Add support for Heroku Redis
@singingwolfboy singingwolfboy merged commit 1962cbb into heroku-python:master Mar 13, 2016
@brainix
Copy link

brainix commented Mar 14, 2016

I apologize but I haven't had a chance to get to my PR yet. I'm still going to create a new PR - and I'll do it ASAP, as I know that the release is waiting! Thanks.

@singingwolfboy
Copy link
Contributor

@brainix: no worries, I know how life can get in the way. 😄 There's no hurry. If I don't get a PR from you in a few days, I'll cut a release without it, but we can always make another release after your PR gets in.

@brainix
Copy link

brainix commented Mar 14, 2016

Thanks for understanding!

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

Successfully merging this pull request may close these issues.

3 participants