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

Mirage support #67

Closed
brendanlong opened this issue Apr 24, 2020 · 11 comments · Fixed by #78
Closed

Mirage support #67

brendanlong opened this issue Apr 24, 2020 · 11 comments · Fixed by #78

Comments

@brendanlong
Copy link
Contributor

#57 and #58 are both attempts by @paurkedal to support Mirage. The point of this ticket is to:

  1. Get Mirage support working
  2. Ensure it stays working

For Mirage support, I'm wondering if this would be Pgx_unix, or another new Mirage-specific one?

Is there a way we can run a CircleCI build to ensure this works?

@brendanlong
Copy link
Contributor Author

I asked about this on the OCaml Discuss forum: https://discuss.ocaml.org/t/easiest-way-to-ensure-a-package-is-usable-in-mirage-in-ci/5607

@paurkedal
Copy link
Contributor

I should mention that the pgx master branch works with MirageOS. I am wondering if, after improving the API, you would be open to a PR adding a pgx-mirage subpackage containing the adapter and a simple test, which at least does a non-unix build?

@anuragsoni
Copy link
Contributor

anuragsoni commented Apr 25, 2020

@paurkedal I was also working to add a pgx-mirage subpackage, but if you already have an implementation working, we'd be very interested in merging support for mirage!

@paurkedal
Copy link
Contributor

@anuragsoni Depends how far you got. If you look at the discuss thread, you'll see I'm missing functoria support, but it's otherwise working (well, at least a simple query). I posted by version in a gist, so you can compare.

@anuragsoni
Copy link
Contributor

anuragsoni commented Apr 27, 2020

@paurkedal I just looked at your gist, that's pretty much what i had as well (modulo some differences about at what point the channel was flushed. I also was leaning towards failing for let getlogin () = Lwt.return "unikernel" (* TODO: Other default or fail? *)

I missed the update on the discuss thread yesterday, and functoria indeed looks like the way to go here. I'm still pretty early in my mirage journey so I didn't think about functoria as for some reason I assumed that's more for use by the unikernel application vs a library that's usable via a unikernel.
I should look at the project layout for some existing mirage libraries like conduit as that should help me learn how a library should be structured to make it easy to consume via the regular mirage workflow.

@paurkedal
Copy link
Contributor

Good, I suggest you pull ahead with your implementation then.

I think an option for getlogin, and maybe even all of host, port, user, password, and database, would be to make them configurable via the MirageOS config.ml. But otherwise I agree, since just having a user without other settings will hardly be useful for remote connections, which always requires at least a at host, but which should also be authenticated.

@anuragsoni
Copy link
Contributor

@paurkedal Sounds good. I'll add an initial implementation soon just to get the ball rolling, while i learn a little more about how to improve the library via functoria.

There are a couple of points in my implementation (around opening the connection) that i'd like to adapt based on your gist. I was curious about what's the LICENSE for the gist?

@paurkedal
Copy link
Contributor

Maybe it's simplest if I make it public domain, just checking what's the right way to do it.

@paurkedal
Copy link
Contributor

I updated the draft with an LGPL 2-or-later license header, as it wasn't clear to me how to make the public domains statement, but I can assign over copyright or make it public domain if you know the procedure.

@anuragsoni
Copy link
Contributor

I updated the draft with an LGPL 2-or-later license header, as it wasn't clear to me how to make the public domains statement, but I can assign over copyright or make it public domain if you know the procedure.

Would this need to be LGPL 2 with the OCaml linking exception? That's the current license for pgx. I'm guessing this exception is present because of OCaml statically linking by default? I'd appreciate any guidance here since I've mostly dealt with MIT/BSD license before.

@paurkedal
Copy link
Contributor

Fixed. @brendanlong I copied the header from pgx/src/pgx.mli, so there may be a slight licensing issue of the original PG'OCaml code, just so you know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants