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

Allow socket usage in DATABASE_URL #71

Merged
merged 1 commit into from
Mar 9, 2024
Merged

Allow socket usage in DATABASE_URL #71

merged 1 commit into from
Mar 9, 2024

Conversation

dlackty
Copy link
Contributor

@dlackty dlackty commented Mar 1, 2024

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.

This is a backport of rails/rails#50349

@@ -155,6 +155,12 @@ def initialize_type_map(m)
end

def initialize(connection, logger, connection_options, config)
config = config.dup

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

@lorint lorint Mar 1, 2024

Choose a reason for hiding this comment

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

Missing a closing back-tick after "host". Should be:

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Anyway, I shouldn't worry -- the same typo is also in the original!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated commit to fix the typo. Thanks for pointing this out!

Copy link
Collaborator

@lorint lorint left a comment

Choose a reason for hiding this comment

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

Looking forward to having this patch in place as it will fix #64 and #67.

Copy link
Collaborator

@lorint lorint left a comment

Choose a reason for hiding this comment

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

Thank you kindly for bringing this patch to ARTA.

@bensheldon bensheldon merged commit 1f91435 into trilogy-libraries:main Mar 9, 2024
9 checks passed
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.

3 participants