search

underyx / flask-redis

A Flask extension for using Redis
Other
436 stars 71 forks source link

Registered flask_redis as an extension with the Flask app and mocked out the redis client #14

Closed csueiras closed 8 years ago

csueiras commented 9 years ago

@rhyselsmore

I needed to have the extension registered with the app, so that we can do:

current_app.extensions['redis'].get("myrediskey")

As recommended by Flask's docs (http://flask.pocoo.org/docs/0.10/api/) look at extensions property of Flask.

Besides this change, I mocked out the redis client so that the unit tests don't connect to Redis.

The requirements did not have flask or redis, so I added them to the pip's requirements.txt file as well.

underyx commented 9 years ago

Hey there, thanks for your contribution!

Registering as an extension did make it into the v0.1.0 release I just uploaded, but I messed up the rebasing somehow, and the extension changes went into 18892582802c020de05c1773ddd0df692cf90230 instead of your own commit. Sorry about that!

I'm keeping this pull request open, since the other half of it has not been merged yet.

underyx commented 8 years ago

So, I got around to reviewing this, finally. I've reworked the project to separate integration and unit tests, and used pytest-mock instead of your solution for the unit tests. I've also kept the existing tests unmocked, since I believe they're more useful as full-blown integration tests. The unit test package is pretty bare-bones right now, with only one example test in it.

So even though most of your contributions ended up unused (sorry :() I did salvage the Flask extension tests that you included here, and I've attributed them to you in a separate new commit, see here: https://github.com/underyx/flask-redis/commit/98979af37f1c8f3fa566aa609fdecc9924a4f8ca

I'm sorry it took so long to get this done, especially since this seems like it might've been one of your first contributions to open source. Hope your next attempts will end up being treated a lot better, and thanks a lot for this pull request!