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

Format of Databasehost for Sockets #35884

Closed
wants to merge 3 commits into from
Closed

Format of Databasehost for Sockets #35884

wants to merge 3 commits into from

Conversation

obel1x
Copy link

@obel1x obel1x commented Dec 26, 2022

Summary

First, the Documentation was wrong for the dbhost in config.php when using sockets and second, made the installer accept the filename without adding "localhost:" in front, which is uncommon so not intuitive and not documented.

TODO

nothing.

Checklist

X Code is properly formatted
X Sign-off message is added to all commits

  • [?] Tests (unit, integration, api and/or acceptance) are included
  • [-] Screenshots before/after for front-end changes
  • Documentation (manuals or wiki) has been updated or is not required
  • [?] Backports requested where applicable (ex: critical bugfixes)

Makes the installer accept Unix- Sockets directly.

Signed-off-by: Daniel Pd <obel1x@web.de>
the format of unix-sockets needs to be localhost:socketfile

Signed-off-by: Daniel Pd <obel1x@web.de>
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Dec 26, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Dec 26, 2022
@obel1x
Copy link
Author

obel1x commented Dec 26, 2022

@kesselb this redoes my broken pull #35379 . Hope now its fine and thank you for your patience, i am getting used to git and eclipse slowly ;)

@kesselb
Copy link
Contributor

kesselb commented Dec 27, 2022

Thank you @obel1x

Functionality also implemented in ConnectionFactory.

Signed-off-by: Daniel Pd <obel1x@web.de>
@obel1x
Copy link
Author

obel1x commented Dec 27, 2022

turned out to be more tricky, as the value is written to config and needs to be read from there in the same style. So now it should both work in config which makes it more styled like in other configs and better for new users. Those should both work e.g.:

  • 'dbhost' => 'localhost:/run/mysql/mysql.sock' (old style)
  • 'dbhost' => '/run/mysql/mysql.sock' (new optional style)

@blizzz blizzz mentioned this pull request Feb 1, 2023
@obel1x obel1x closed this by deleting the head repository Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In the initial setup, entering an UNIX socket for the database host returns error
3 participants