all: add cockroachdb support

[lopezator]

add cockroachdb support

Signed-off-by: David López not4rent@gmail.com

Related PR

#1326

[lopezator]

@aeneasr I cannot figure out where is the problem here with the tests, can you give me a hand with this?

[lopezator]

Related commit in x: lopezator/x@44fa603

[aeneasr]

Thank you for all your hard work! The most recent error seems to be stemming from broken SSL?

--- FAIL: TestXXMigrations (302.66s)
    helper.go:58: Could not connect to database: Unable to connect to postgres (cockroach://root@localhost:26257/defaultdb?sslmode=disable): pq: SSL is not enabled on the server

[lopezator]

I've been doing some advances on this and been able to pass almost all tests, but still some failing. I've been working on my fork to avoid spamming you. It's a draft implementation though:

If you could take a look, it would be great, to collect some feedback.

lopezator@d7ce922

I'll keep you posted if I make some significative advance.

[aeneasr]

I think it's only a couple of copy&paste errors in the circle config. Sometimes you're using postgres instead of cockroach which probably causes weirdness when connecting. Left some comments on your commit.

[aeneasr]

So this is basically ready for a final review and then merge (after resolving conflicts)?

[lopezator]

Not yet.

I wanted to clean-up some code (ifs) I added for the cockroachdb/postgres dsn overrides etc...

Anyway, correct me If I am wrong but maybe I should first send a PR to ory/x, and then you review/merge it before this, as it doesn't break anything (just adds cockroach compat to x), in order to remove the replace statement from the go.mod file and point to a proper version of ory/x here:

https://github.com/ory/hydra/pull/1348/files#diff-37aff102a57d3d7b797f152915a6dc16R88

I'm working now on passing tests and cleaning up things on x here:

lopezator/x@1b7f14d

I'll send you a PR there when I'm done.

I also opened this issue in sqlx we'll see what happens, because it would help to have a cleaner code removing the need of wrapping some sqlx functionality:

jmoiron/sqlx#511

[aeneasr]

#1423

[aeneasr]

Just a few more changes then it should be good to go!

[lopezator]

PTAL @aeneasr

I changed all the cockroach sql file numbers to match the postgres and the mysql migrations number, and since the tests are just inserts and now share the same index, we can safely delete the cockroach test migration sql files/folders.

I originally reordered some columns on cockroach create table migrations for readability, e.g. (pk as first column, created_at and updated_at as latest columns...). I changed that. Now the columns follow the same exact order as mysql and postgres "finished" databases, so the comparison against them is easier.

[aeneasr]

Awesome! I think that's it - once tests pass this will be merged. Thank you for this immense effort!

[aeneasr]

I think tests fail because we're using the same shared test sqls!

[lopezator]

Just for the record, we freeze this until the underlying required change on ory/x got merged to correlate migration sql file ids, and test sql file ids.

Then, just pointing to new ory/x should fix the failing tests.

Just FYI, we packed lopezator/hydra in a container and deployed it on our cloud (to test out the cockroach support), and AFAIK it's working great.

[aeneasr]

Just FYI, we packed lopezator/hydra in a container and deployed it on our cloud (to test out the cockroach support), and AFAIK it's working great.

That's great news!!!

[lopezator]

PTAL @aeneasr

Pending:

1. Remove the go.mod replace after merging migratest: add FindMatchingTestMigrations func x#52
2. Squash the second commit: 50ea156 into the first one, I've left it like this for easier review as it's a big PR to focus only on what changed.

[lopezator]

go.mod updated, rebase&squash done. Almost there! 😅

[aeneasr]

😎

[aeneasr]

Thank you!

[lopezator]

Thank you for merging and supporting me during the whole proccess! 🎉 🎉