Skip to content

Add Ecto Type for IP Addresses#11

Closed
aj-foster wants to merge 2 commits into
mainfrom
aj/ums-492-support-ip-address-types-in-ecto
Closed

Add Ecto Type for IP Addresses#11
aj-foster wants to merge 2 commits into
mainfrom
aj/ums-492-support-ip-address-types-in-ecto

Conversation

@aj-foster
Copy link
Copy Markdown
Collaborator

This PR adds an Ecto type for conveniently casting and storing IP addresses. It also installs sobelow and adds a mix check alias.

@aj-foster aj-foster requested review from a team as code owners August 9, 2025 03:04
Copy link
Copy Markdown
Contributor

@PragTob PragTob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, but I'd like to see an argument why we choose to implement our own over using ecto_network instead.

Otherwise looks good according to my understanding (other than us not specifying :inet as type) - since we're recommending to store it as :inet I'd love a test showcasing storing and retrieving it successfully from such a column - but I can also see how people would think that's overkill.

Comment thread lib/together/ip.ex

@doc false
@impl Ecto.Type
def type, do: :string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never done this, but why are we specifying :string here instead of :inet?

It should also be possible, we might also consider instead using INET from ecto_network

Comment thread test/together/ip_test.exs
@aj-foster
Copy link
Copy Markdown
Collaborator Author

Nevermind, just use EctoNetwork.INET.

@aj-foster aj-foster closed this Aug 11, 2025
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

Successfully merging this pull request may close these issues.

2 participants