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

Fix: detect IPv6 and use appropriate socket for Realtime.Repo #1170

Closed
wants to merge 3 commits into from

Conversation

tvogel
Copy link

@tvogel tvogel commented Oct 18, 2024

What kind of change does this PR introduce?

This change allows to use a DB_HOST connected via IPv6.

What is the current behavior?

When the DB_HOST resolves to an IPv6, connecting to the database fails with :nxdomain.

What is the new behavior?

The connection is established successfully.

Additional context

Uses logic from Realtime.Database.detect_ip_version/1 and applies it to the Realtime.Repo socket_options, too. Unfortunately, I was not able to directly call Realtime.Database.detect_ip_version/1 from runtime.exs. I am an Elixir newbie, so there may be a more elegant way.

Copy link

vercel bot commented Oct 18, 2024

@tvogel is attempting to deploy a commit to the Supabase Team on Vercel.

A member of the Team first needs to authorize it.

@filipecabaco
Copy link
Contributor

this is already automatically by Realtime.Database.detect_ip_version/1 and we already run it in production with IPv6

is your use case not covered by this?

@tvogel
Copy link
Author

tvogel commented Oct 29, 2024

this is already automatically by Realtime.Database.detect_ip_version/1 and we already run it in production with IPv6

is your use case not covered by this?

Right, that only buys part of IPv6 support as described above: That detection is also needed when setting the socket-options for the Realtime.Repo. As written above, I got an :nxdomain in a pure IPv6 environment without these changes.

@filipecabaco
Copy link
Contributor

oh got it! what do you think of making an explicit check for the repo?

e.g.

environment variable determining that the root db will be IPV6 true / false and use that to set things? Less code and easier to understand what is happening

tenants required to be more dynamic but the base database should be more "set in stone"

@filipecabaco
Copy link
Contributor

we'll also need to update the mix.exs file to bump up the version 👀

use logic from Realtime.Database.detect_ip_version() and apply it to the
Realtime.Repo socket_options, too;
@tvogel tvogel force-pushed the tv/fix-ipv6-db-host branch from 50b83ce to 32bc2e5 Compare November 4, 2024 10:20
@tvogel
Copy link
Author

tvogel commented Nov 14, 2024

we'll also need to update the mix.exs file to bump up the version 👀

How do you handle the versions? I have updated it before but obviously, it's a race vs. other PRs. How to proceed?

@filipecabaco
Copy link
Contributor

we had a couple of tricky weeks, hopefully after some of the pending PRs are tackled I will grab this and merge it myself. I hope to do it within a week

sorry for the inconvenience 😞 just really intense weeks overall

@filipecabaco
Copy link
Contributor

Sorry for the delay on this PR, I've created this one so we can proceed faster

#1250

@filipecabaco
Copy link
Contributor

the PR has been merged, thank you again for bringing this issue up @tvogel 🙏 and big sorry for the time it took to tackle it

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