pgx mirage #78
pgx mirage #78
Conversation
|
Tested manually by running the example unikernel against a postgres instance running on google cloud. |
Create the data before fetching it
anuragsoni
force-pushed
the
try-pgx-mirage
branch
3 times, most recently
from
5a8a32c
to
1232d90
Compare
pgx_lwt/src/s.ml
Outdated
|
|
||
| module type Pgx_impl = sig | ||
| include Pgx.S with type 'a IO.t = 'a Lwt.t | ||
| end |
There was a problem hiding this comment.
Why do we need this? Isn't this just Pgx.S with type IO.t := Lwt.t?
There was a problem hiding this comment.
Good catch. I had initially done this as i need to return a first class module from the pgx_lwt_mirage.connect function and parameterized types aren't supported for packed modules. But I didn't realize that I can just create a temporary alias inside the mirage backend an then i won't need to create this temporary instance. I'll update the PR
There was a problem hiding this comment.
The module alias is now just Pgx_lwt.S and is shared across the different lwt backends
| @@ -0,0 +1,28 @@ | |||
| (* Draft for cherry-picking into pgx. | |||
There was a problem hiding this comment.
I think this came from when i copied the license header from the gist linked to in the issue for mirage. I wasn't sure what content i'm allowed to touch in such situation
There was a problem hiding this comment.
We can edit this comment as long as we leave the license block alone and only add to the copyright section.
There was a problem hiding this comment.
Updated the header to remove the comment about cherry-picking
pgx_test/src/dune
Outdated
| @@ -1,3 +1,3 @@ | |||
| (library | |||
| (name pgx_test) | |||
| (libraries alcotest base64 pgx)) | |||
| (libraries alcotest base64 pgx pgx_value_core)) | |||
There was a problem hiding this comment.
Why does Pgx_test need a dependency on this? I thought Pgx_value_core handled its own testing.
There was a problem hiding this comment.
this should be removed. I'll update
There was a problem hiding this comment.
This is now removed
We should allow pgx to be used on Lwt versions < 5.3
pgx/src/pgx.ml
Outdated
| @@ -471,7 +471,8 @@ module type IO = Io_intf.S | |||
| module type S = Pgx_intf.S | |||
|
|
|||
| module Make (Thread : IO) = struct | |||
| open Thread | |||
| module IO = Thread | |||
| open IO | |||
There was a problem hiding this comment.
What's the reason for this change? It looks like it creates a bunch of downstream complexity (like the Pgx_lwt.Io_intf module) but doesn't seem to change anything else?
There was a problem hiding this comment.
It looks like we did this to accomidate Pgx_lwt, but I think we should keep our interface here simple and let Pgx_lwt do more complicated things to generate the interface if necessary. I'll try to make an example.
TODO
Closes #67