search

krisprice / ipnet

IpNet, Ipv4Net, and Ipv6Net types and methods for Rust
Apache License 2.0
122 stars 26 forks source link

Interest in Diesel support? #42

Closed smklein closed 1 year ago

smklein commented 2 years ago

This is a continuation of the discussion originally posted here: https://github.com/diesel-rs/diesel/discussions/3164

TL;DR: I like Diesel, I like ipnet, I want them to work well together! However, this leaves the question: where should the bindings to allow IpNet compatibility with Diesel live?

Diesel is a crate which provides a database query builder in Rust. It makes heavy use of the FromSql and ToSql traits to convert "rust types" to/from "sql types". I have an interest in implementing this type for the IpNet object itself.

Actually implementing this trait is relatively straightforward, but there's an open question which I raised in the Diesel discussion - should this be a part of the ipnet crate (behind a "diesel" feature flag) or a part of the diesel crate (behind an "ipnet" feature flag)?

How do y'all feel about this? As some prior art:

krisprice commented 2 years ago

Thanks @smklein - thinking on this I can see good arguments for putting support in both locations. But the intention for IpNet has always been to remain fairly small, low-ish level. So from that perspective without knowing how many IpNet users out there would want diesel support it seems better to add support in diesel if they're open to that?

weiznich commented 2 years ago

But the intention for IpNet has always been to remain fairly small, low-ish level. So from that perspective without knowing how many IpNet users out there would want diesel support it seems better to add support in diesel if they're open to that?

Speaking as a diesel maintainer here. A similar argument applies there as well. This support would likely be only useful for a small fraction of our users, so I would be quite happy to not having this code in diesel itself. Unfortunately there is no other convenient option than putting that support into ipnet or into diesel. At least for a potential inclusion into diesel it would be interesting to know how often you plan to release breaking changes?

krisprice commented 2 years ago

At least for a potential inclusion into diesel it would be interesting to know how often you plan to release breaking changes?

Intention is zero breaking changes. The structure itself and it's existing serialization implementation shouldn't ever change. There may be a version 3 at some point if say it ever made sense to represent the output of hosts() as a Range. But that's looking unlikely.

krisprice commented 1 year ago

I see @weiznich went ahead with merging into diesel a few months ago. Thanks for that @smklein and @weiznich.