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

Failure to connect with Cockroach serverless uri #95

Closed
dangdennis opened this issue Sep 18, 2022 · 7 comments
Closed

Failure to connect with Cockroach serverless uri #95

dangdennis opened this issue Sep 18, 2022 · 7 comments

Comments

@dangdennis
Copy link

dangdennis commented Sep 18, 2022

Issue

Caqti cannot connect to cockroachdb serverless's uri. I suspect it might be that a particular query param is filtered out during uri parsing.

Example

Attempting to connect to cockroach's url at postgresql://user:password@free-tier14.aws-us-east-1.cockroachlabs.cloud:26257/defaultdb?sslmode=verify-full&options=--cluster=some-cluster-name-4321 yields

18.09.22 21:27:43.110       dream.log  WARN Called from Caqti_driver_postgresql.Connect_functor.connect.(fun) in file "caqti-driver-postgresql/lib/caqti_driver_postg1
18.09.22 21:27:43.110      dream.http ERROR REQ 4 Failed to connect to <postgresql://dennis:_@free-tier14.aws-us-east-1.cockroachlabs.cloud:26257/defaultdb?sslmode=v"

18.09.22 21:27:43.110      dream.http ERROR REQ 4 Raised at Stdlib__Map.Make.find in file "map.ml", line 137, characters 10-25
18.09.22 21:27:43.110      dream.http ERROR REQ 4 Called from Logs.Tag.find in file "src/logs.ml", line 154, characters 14-32

Notice that <postgresql://dennis:_@free-tier14.aws-us-east-1.cockroachlabs.cloud:26257/defaultdb?sslmode=v" misses the expected &options=--cluster=some-cluster-name-4321.

Anyone can create a new cluster for free at https://cockroachlabs.cloud/

If I resolve the issue, I'll create a PR

@dangdennis
Copy link
Author

dangdennis commented Sep 18, 2022

Running the cmd in utop confirms it's some failure in parsing of the options key. If we replace the extra = with its unicode %3D, I still get the same error. Hmm

module Db = (val Caqti_lwt.connect (Uri.of_string "postgresql://dennis:YKXpVzPorCaKY2NaaJH1xw@free-tier14.aws-us-east-1.cockroachlabs.cloud:26257/defaultdb?sslmode=verify-full&options=--cluster=saucy-gundi-4894") >>= Caqti_lwt.or_fail |> Lwt_main.run);;
Failed to connect to <postgresql://dennis:_@free-tier14.aws-us-east-1.cockroachlabs.cloud:26257/defaultdb?sslmode=verify-full&options=--cluster=saucy-gundi-4894>: Connection failure: extra key/value separator "=" in URI query parameter: "options"

But parsing the uri itself yields the KV params correctly. The = in options is already decoded.

utop # Uri.query b
;;
- : (string * string list) list =
[("sslmode", ["verify-full"]); ("options", ["--cluster=saucy-gundi-4894"])]

@paurkedal
Copy link
Owner

paurkedal commented Sep 20, 2022

Are you sure the option and argument shall be separated by a = and not a space (%20)? As for as I can tell, the error comes from either libpq or the remote side, since Caqti passes the URI to ocaml-postgresql which passes it to libpq.

@paurkedal
Copy link
Owner

Forget my previous post, the problem is rather than the URI-encoding is lost when going though Uri.of_string and Uri.to_string, but it can probably be customized with the Uri.pct_encoder.

@paurkedal
Copy link
Owner

Can you check if 867049e (on master) fixes the issue?

@dangdennis
Copy link
Author

dangdennis commented Sep 20, 2022

I'll check this out today. Thank you for the commit @paurkedal.

Aside, can you explain how you locally iterate on the ocaml library? I couldn't figure out the workflow.

My best attempt was to use opam to pin caqti and caqti-driver-postgresql to the local clone of this repo such that my app would supposedly use the locally pinned repo with my changes. That didn't work seem to work. Edits to the local files didn't get picked up, but I suspect it's some minor issue.

On the other hand, the real bugger was that I couldn't get the caqti-driver-postgresql tests to yield any real test results with dune runtest caqti-driver-postgresql -f beyond

Testing `test_postgresql'.
This run has ID `W3JIEGJ5'.


Full test results in `~/Desktop/ocaml-caqti/_build/default/caqti-driver-postgresql/test/_build/_tests/test_postgresql'.
Test Successful in 0.000s. 0 test run.

@paurkedal
Copy link
Owner

To your first question, opam pin add caqti-driver-postgresql --dev-repo should work, since it's still compatible with the released packages. In general, you might have to pin all Caqti packages you use.

To the second question, the testsuite will only test against URIs listed in the file testsuite/uris.conf, which defaults to sqlite3-only. Put one URI per line.

@dangdennis
Copy link
Author

It works! I connected to my serverless crdb cluster and successfully queried some data.

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

No branches or pull requests

2 participants