-
Notifications
You must be signed in to change notification settings - Fork 185
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
Support connecting with a unix domain socket #286
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for putting this in.
A few comments:
- The docs should be updated, specifically the main README and
docs/configuration.md
- We should have some kind of test for this, even just a unit test of
Connection.MySQL()
- Should
--port
be deprecated, and everything move into--server
? So you can do--server 127.0.01:3306
or--server 127.0.0.1
or--server /path/to/unix
, and `--port will be ignored?
bb1708e
to
2468197
Compare
Done!
I'm afraid I'm not much a go-lang user myself so I don't feel comfortable adding this, would you be able to add the test? (If there was already a similar test I could modify it).
I don't think that's necessary. Port is already optional, and this change preserves that. Since the go-sql-driver does accept I leave that decision up to you as the maintainer. |
I've enabled the "Allow edits by maintainers" feature so feel free to push to my branch if you want. |
This looks good to me. I won't be able to add enough for the testing for the next week at least, and don't want to hold it up. Existing testing should catch any errors for current functionality, and unix-domain socket is new, so I am good with that. I approved it. Anything else you want before we merge it in? |
I did run a local manual test to ensure that I didn't break the tcp connect functionality.. both unix and tcp are working as expected.
Nope, nothing for this commit. Good to merge. I've noticed a few documentation inconsistencies that I think are a result of the bash -> golang switch, but I'll open another issue for those. |
As discussed in #285
This changes the semantics of
--server
(or the env var, etc) to--server <IP/FQDN | /path/to/socket>