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

TrilogyAdapter: ignore host if socket is set #50349

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

casperisfine
Copy link
Contributor

Contrary to mysql2, trilogy will ignore the socket config if host is set.

This makes it impossible to configure a UNIX domain socket connection via DATABASE_URL, as the URL must include a host.

Contrary to mysql2, trilogy will ignore the `socket` config
if `host` is set.

This makes it impossible to configure a UNIX domain socket connection
via `DATABASE_URL`, as the URL must include a host.
@@ -76,6 +76,10 @@ def initialize_type_map(m)
def initialize(config, *)
config = config.dup

# Trilogy ignore `socket` if `host is set. We want the opposite to allow
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Trilogy ignore `socket` if `host is set. We want the opposite to allow
# Trilogy ignores `socket` if `host` is set. We want the opposite to allow

Copy link
Member

Choose a reason for hiding this comment

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

This was fixed in ee65e97 on main, if someone wants to backport it 🙇 /cc @byroot

@byroot byroot merged commit bec5cde into rails:main Dec 13, 2023
3 of 4 checks passed
byroot added a commit that referenced this pull request Dec 13, 2023
TrilogyAdapter: ignore host if socket is set
@byroot
Copy link
Member

byroot commented Dec 13, 2023

Backported to 7-1-stable as 377650f (I consider this a bug given it's something that was possible with mysql2, but not trigloy).

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

Successfully merging this pull request may close these issues.

None yet

4 participants