Skip to content

Add options for database string and use it for sockets #7

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

witchent
Copy link

I wanted to use your script with a unix socket to connect to mariadb. Most stuff works, but to create the correct tables for the databases I needed to adapt a few things.
Instead of only extracting the unix_socket string, I decided to make it more logical by extracting all options of the mysql:// string and then only use the unix_socket right now. But if in the future one wants to e.g. also accept ssl connections this would now be an easy change.

What I don't like is copying the database-command, so if you have a better idea please feel free to adapt it.
If you don't want to support this usecase this is also fine, then just close it, at least next time somebody wants to do it they can find this PR and see how it could be done.

I tested that change with tcp connection and unix_socket connection and both work fine now.

@dan-r
Copy link
Owner

dan-r commented Apr 15, 2025

Thanks for your contribution - is there a clear reason for this approach vs. just setting the host to localhost?
IIRC that makes mysql clients connect with a local socket, although I'm not sure on the default location for this.

@witchent
Copy link
Author

witchent commented Apr 15, 2025

Well its twofold:

  1. We need the if-then-else part anyway because otherwise it fails because the port option is still used
  2. I then thought about just making the if-then-else about host and port and otherwise use aforementioned default fallback, but then I thought why not allow setting a custom socket path if I have to change something anyhow (because I reuse a lot of my container configuration I always have the socket at a custom path).

So I extracted the unix_socket path from the connection string, and then I decided to just extract all options in case at one point someone wants to use other options (like ssl termination etc). This is a very small step up but might make stuff in the future easier.

That's how this PR was created. Again, if you don't agree with it I am fine with you closing it/reworking it to just check for empty ports, but I thought I might as well propose it in public, so at least people searching for this problem know how to fix it.
Already super happy this project exists at all, so thanks for this :)

Edit:// As for why allowing custom paths could be useful: Apart from my reason (reusing container configurations), I know that some people like to use two mariadb instances for the two databases. Both cannot listen on the same default socket in the syncstorage-container, so one of them has to listen on a non-default socket, making the fallback not work.

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