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

Add Date and Time Pgx.Values for Lwt backend #73

Closed
anuragsoni opened this issue Apr 27, 2020 · 4 comments
Closed

Add Date and Time Pgx.Values for Lwt backend #73

anuragsoni opened this issue Apr 27, 2020 · 4 comments

Comments

@anuragsoni
Copy link
Contributor

While prototyping a mirage backend for pgx (#67) I noticed that the Date/Time convertors are missing (they are present in the async backend).

For the lwt runtime maybe we can look at ptime (Calendar depends on unix so won't be usable on mirage) to provide similar support to create Pgx values?

In theory we could use the same functions as we do for the async runtime since they are using the date/time values from core_kernel which should be portable, but i'm not sure it'd be worth it to pull in core_kernel for the lwt runtime as i'm not sure if many people would use that combination.

@brendanlong brendanlong changed the title Date and Time Pgx_values missing for lwt backend Add Date and Time Pgx.Values for Lwt backend Apr 27, 2020
@brendanlong
Copy link
Contributor

Yeah, the reason we only expose it for Pgx_async is that we didn't want to push the full Core dependency on everyone (I'm not sure if Core_kernel even existed at the time).

I wonder if we could write the Pgx_value modules in a way where people can just depend on the ones they want, and then provide pgx_values_core.opam, pgx_values_ptime.opam, etc.

brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
brendanlong added a commit that referenced this issue Apr 27, 2020
This will let people using the Lwt or Unix backends still
have access to the Core_kernel.{Date,Time} converters.

See #73
@brendanlong
Copy link
Contributor

After #75, it's possible to use Pgx_lwt + Pgx_value_core. I think a Pgx_value_ptime would still be a good idea though.

@paurkedal
Copy link
Contributor

Yes, ptime is a very minimal library and also quite popular, so I think that would be a good addition.

@brendanlong
Copy link
Contributor

I'm going to close this as complete since we have Pgx_value_core now, and I opened #97 for ptime.

@brendanlong brendanlong self-assigned this May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants