Closed leamingrad closed 5 years ago
Looks good. (Registering twice is okay, since it's just a Hashtbl
entry. One could add an ?aliases
argument to the registration function, but it would do the same thing. The scheme translation breaks the idea that drivers should be fully definable from external packages, but I don't see a simple solution, so let's stick with it.)
Can you tweak your PR a bit before I merge?:
let scheme_driver_name = function
| "postgresql" | "postgres" -> "caqti-driver-postgresql"
| s -> "caqti-driver-" ^ s
Made the changes above - I didn't raise an issue for this so I don't have a number, but I'm happy to raise one and reference it if you would like.
Looks good, thanks!
This is a small PR to fix an issue that has been causing us a bit of trouble when trying to use
caqti
.In the postgres documentation here it states that valid postgres URIs can use either the
postgresql
orpostgres
scheme. It seems like thepostgres
scheme is quite common, for example when deploying to heroku.The fix here is to handle the special case when resolving the driver name at connect time, and for the
caqti-driver-postgresql
library to register itself for both schemes.I would be interested in your thoughts/feedback on this, including whether registering the driver twice is indeed the right approach - it is definitely the simplest, but there might be a nicer way.