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

Other dbs #42

Merged
merged 4 commits into from
Jul 9, 2024
Merged

Other dbs #42

merged 4 commits into from
Jul 9, 2024

Conversation

xihh87
Copy link
Contributor

@xihh87 xihh87 commented Jun 21, 2024

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

I allowed the variable CMD_DB_DIALECT to be used to setup the CMD_URL_DB in the hedgedoc service definition.

Benefits of this PR and context:

The only way to customize the DB connection without leaking the database information in a compose file is to set up the variables: FILE__DB_HOST, FILE__DB_NAME, FILE__DB_PASS, FILE__DB_USER.

This however does not allow to use other databases beside MySQL, so I added the option to use the variable CMD_DB_DIALECT which is the specified method according to the hedgedoc documentation.

This defaults to mysql, so if the variable is not set-up, the container will work as of now.

How Has This Been Tested?

I have built my hedgedoc instance using this changes, these changes allowed me to connect to a postgres database without leaking info in the compose file.

Source / References:

https://docs.hedgedoc.org/configuration/

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/hedgedoc/1.9.9-pkg-cc4728f5-dev-8c87e6e975db45c129e33837a77c072e1adb846c-pr-42/index.html
https://ci-tests.linuxserver.io/lspipepr/hedgedoc/1.9.9-pkg-cc4728f5-dev-8c87e6e975db45c129e33837a77c072e1adb846c-pr-42/shellcheck-result.xml

Tag Passed
amd64-1.9.9-pkg-cc4728f5-dev-8c87e6e975db45c129e33837a77c072e1adb846c-pr-42
arm64v8-1.9.9-pkg-cc4728f5-dev-8c87e6e975db45c129e33837a77c072e1adb846c-pr-42

@xihh87
Copy link
Contributor Author

xihh87 commented Jun 25, 2024

@thespad, could you please check pull request #42 for docker-hedgedoc?

I think allowing the use of databases beyond mysql would be generally useful and doesn't need any other change.

The commits for this branch could be squashed.

@thespad
Copy link
Member

thespad commented Jun 26, 2024

Can you please explicitly list the possible options in the env description:

mysql, mariadb, sqlite, postgres

And while we're at it we should probably update the default to mariadb in the servicefile, as in most practical cases now that's what people are going to be using.

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/hedgedoc/1.9.9-pkg-cdbf7e6f-dev-21532170fb9bb3622d9513e0aeab0ea023898d4f-pr-42/index.html
https://ci-tests.linuxserver.io/lspipepr/hedgedoc/1.9.9-pkg-cdbf7e6f-dev-21532170fb9bb3622d9513e0aeab0ea023898d4f-pr-42/shellcheck-result.xml

Tag Passed
amd64-1.9.9-pkg-cdbf7e6f-dev-21532170fb9bb3622d9513e0aeab0ea023898d4f-pr-42
arm64v8-1.9.9-pkg-cdbf7e6f-dev-21532170fb9bb3622d9513e0aeab0ea023898d4f-pr-42

@thespad thespad merged commit 3174be6 into linuxserver:main Jul 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants