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

handle db-url #3178

Merged
Merged

Conversation

winston0410
Copy link
Contributor

@winston0410 winston0410 commented Aug 6, 2024

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.
For #3172

This PR implemented a new field in configuration called db-url. This allow user to defined their connection to database using DSN, so they can customize the database behavior as much as they want. For example, they can allow connection to DB with schema using the following URL:

db-url: 'postgres://user:pass@localhost/db?search_path=gotosocial'

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have not leveraged AI to create the proposed changes.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@winston0410
Copy link
Contributor Author

Hi @tsmethurst , do you mind reviewing this for me please?

@tsmethurst
Copy link
Contributor

Hmm I think I'd rather we called this db-postgres-connection-string, and put it after the db-sqlite-* options in the config, just to emphasize that it's an advanced option and specific to PG. Could you make those changes? Aside from that this looks good to me I think :)

@winston0410 winston0410 force-pushed the feat/handle-db-extra branch from 9d5d1e8 to 3294678 Compare August 8, 2024 10:59
@winston0410
Copy link
Contributor Author

@tsmethurst no problem, I have changed from db-url to db-postgres-connection-string now. Can you review this again?

@tsmethurst
Copy link
Contributor

Looks good, thank you!

@tsmethurst tsmethurst merged commit 94c615d into superseriousbusiness:main Aug 8, 2024
2 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.

2 participants