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

Add Redis backend #8

Merged
merged 1 commit into from
Jan 8, 2017
Merged

Add Redis backend #8

merged 1 commit into from
Jan 8, 2017

Conversation

talex5
Copy link
Collaborator

@talex5 talex5 commented Dec 5, 2016

Notes:

  • The test code checks that it gets "Expired" for expired keys. But keeping expired keys around seems pointless, so for the Redis checks I allow it to return Not_found instead.

  • The tests assume you have a Redis server running on 127.0.0.1, which probably isn't reasonable. Maybe they should be disabled. How does it work for Postgres?

Fixes #3.

Has only been very lightly tested so far.

@talex5
Copy link
Collaborator Author

talex5 commented Dec 6, 2016

A few updates:

  • I've added a new flag to enable the Redis tests: ./configure --enable-redis-tests.
  • I use a connection pool rather than a single connection, so it can recover if the connection is lost and can run queries in parallel.
  • Added a prefix to the session keys in case the DB is being used for other things too.

@talex5
Copy link
Collaborator Author

talex5 commented Dec 28, 2016

We've been using this for a few weeks now and it seems to be working well.

Note:

- The test code checks that it gets "Expired" for expired keys. But
  keeping expired keys around seems pointless, so for the Redis checks
  I allow it to return Not_found instead.
@talex5
Copy link
Collaborator Author

talex5 commented Jan 4, 2017

Would be good to get this merged, because it's a dependency of datakit-ci and we can't release that to opam-repository until this is released.

@seliopou seliopou merged commit 6db30f7 into inhabitedtype:master Jan 8, 2017
@avsm
Copy link
Contributor

avsm commented Jan 13, 2017

Thanks for merging! Any chance of a release in opam-repository stable containing this, so that we can release the CI software as well?

@seliopou
Copy link
Member

See ocaml/opam-repository#8374

@talex5 talex5 deleted the redis branch January 30, 2017 13:52
@talex5
Copy link
Collaborator Author

talex5 commented Jan 30, 2017

Thanks! I've updated DataKitCI to use the released version.

@avsm
Copy link
Contributor

avsm commented Jan 30, 2017

thanks @seliopou!

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.

3 participants