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

adding RethinkDB as a Store #39

Closed
joeblew99 opened this issue Dec 29, 2015 · 13 comments
Closed

adding RethinkDB as a Store #39

joeblew99 opened this issue Dec 29, 2015 · 13 comments
Labels
feat New feature or request.

Comments

@joeblew99
Copy link

I want to use this but with rethinkdb.

Anyone have any thoughts on this ?

What i intend to do:

This looks like what needs to be ported.
https://github.com/ory-am/hydra/blob/master/account/postgres/store.go
So just a matter of making a rethinkdb folder, and then porting across store.go.

@joeblew99 joeblew99 changed the title adding extra DB's adding RethinkDB as a Store Dec 29, 2015
@aeneasr
Copy link
Member

aeneasr commented Dec 29, 2015

Yes, all store implementations are abstracted and it should be possible to implement other store backends. I would however do not recommend to use a No-SQL / Non-Acid solution for your auth* services.

RethinkDB, like probably all No-SQL databases, is not ACID compliant. Let's say you revoke a token. Without ACID compliance, the token will be revoked some time in the future. An attacker owning the revoked would therefore still have access although access has been revoked until RethinkDB has propagated the changes across the cluster.

@aeneasr
Copy link
Member

aeneasr commented Dec 29, 2015

If you want to use RethinkDB anyways (e.g. no need for uber-security) there are different stores that need to be implemented for this. For example:

I think most of them are pretty straight forward, you can always take a look at the postgres implementations if you want to.

If you need any help implementing or if you want to try things out feel free to ask me here or on our gitter channel: https://gitter.im/ory-am/hydra?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge

You can also play around with dockertest for rethinkDB integration tests :)

@joeblew99
Copy link
Author

Thanks for the feedback from:
https://www.rethinkdb.com/docs/architecture/

.Write atomicity is supported on a per-document basis – updates to a single JSON document are guaranteed to be atomic. RethinkDB is different from other NoSQL systems in that atomic document updates aren’t limited to a small subset of possible operations – any combination of operations that can be performed on a single document is guaranteed to update the document atomically.

It maybe that this scope if atomicity is enough, but until idig through the DAL / CRUD in the code I can't say.
It all comes down to how relational your design is. If its no too much, its possible to scope everything to a su gle json dic for atomic reason.

@aeneasr
Copy link
Member

aeneasr commented Dec 29, 2015

Hey yep you're absolutely right. I was a little in a hurry and did not read up on everything, sorry for that :)

Atomicity is already one step in the right direction. If you don't manage an uber RethinkDB cluster, it should be a viable storage solution for Hydra. Osin (the OAuth2 library behind Hydra) actually already supports a RethinkDB: https://github.com/ahmet/osin-rethinkdb

You could play around with osin and osin-rethinkdb to get things started. Once that works it shouldn't be hard to get RethinkDB working for Hydra.

@joeblew99
Copy link
Author

It's OK.

Nice. Good idea. Will take a crack at it tomorrow.
But I need all that hydra has, so will have to bite the bullet.

Cheers

On Tue, 29 Dec 2015, 20:30 Aeneas notifications@github.com wrote:

Hey yep you're absolutely right. I was a little in a hurry and did not
read up on everything, sorry for that :)

Atomicity is already one step in the right direction. If you don't manage
an uber RethinkDB cluster, it should be a viable solution for Hydra. Osin
(the OAuth2 library behind Hydra) actually already supports a RethinkDB:
https://github.com/ahmet/osin-rethinkdb

You could play around with osin and osin-rethinkdb to get things started.
Once that works it shouldn't be hard to get RethinkDB working for Hydra.


Reply to this email directly or view it on GitHub
#39 (comment).

@aeneasr aeneasr added the feat New feature or request. label Dec 30, 2015
@leetal
Copy link
Contributor

leetal commented Feb 2, 2016

I'm pro for adding RethinkDB. If you haven't already started working on it, I'll take a bite at it tomorrow. We use RethinkDB in our service backend and has so far not had any problems with atomicity, like pushing changes through the clusters immediately.

@leetal
Copy link
Contributor

leetal commented Feb 8, 2016

Update on this: 40% percent done last night. Will finish after work tonight.

@aeneasr
Copy link
Member

aeneasr commented Feb 8, 2016

You rock!

@joeblew99
Copy link
Author

Hey Alexander. I never got time to do this , so thanks for this. I will be
able to help beta test for sure and contributed back I hope

On Mon, 8 Feb 2016, 10:16 Aeneas notifications@github.com wrote:

You rock!


Reply to this email directly or view it on GitHub
#39 (comment).

@leetal
Copy link
Contributor

leetal commented Feb 8, 2016

So, i didn't finish this today, but most certainly will tomorrow. I only have one function left in ladon (TestFindPoliciesForSubject) to implement but since that function is quite harsh to overview, i'll finish this tomorrow instead so i can get some sleep tonight ;)

@arekkas I think it's time to add rethinkdb support to dockertest as well. Right now all tests for rethinkdb are done locally instead of run in a container.

@leetal
Copy link
Contributor

leetal commented Feb 9, 2016

Ok, i think i'm done now :) Will need some live testing (how the heck i now do that?) and some smaller polishing bits, but at least hydra starts and sets upp all tables without giving any errors!! YAY!

@aeneasr
Copy link
Member

aeneasr commented Feb 9, 2016

good stuff! :)

@joeblew99
Copy link
Author

If you can push and merge I can try it out on Monday

On Tue, 9 Feb 2016, 23:58 Aeneas notifications@github.com wrote:

good stuff! :)


Reply to this email directly or view it on GitHub
#39 (comment).

@leetal leetal mentioned this issue Feb 11, 2016
@aeneasr aeneasr closed this as completed Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request.
Projects
None yet
Development

No branches or pull requests

3 participants