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

feat: add SHIORI_PG_SSLMODE #536

Merged
merged 3 commits into from
Jul 9, 2023
Merged

feat: add SHIORI_PG_SSLMODE #536

merged 3 commits into from
Jul 9, 2023

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Nov 12, 2022

Some managed hosting providers don't support sslmode=disabled.

Some managed hosting providers don't support sslmode=disabled.
Copy link
Member

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

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

Hey @refi64, thanks for the contribution! This looks good to go, though I have made a small comment on the flag check in case you want to clarify that for me. If you think that's good as it is, we can merge this into dev.

@@ -135,8 +135,12 @@ func openPostgreSQLDatabase(ctx context.Context) (database.DB, error) {
user, _ := os.LookupEnv("SHIORI_PG_USER")
password, _ := os.LookupEnv("SHIORI_PG_PASS")
dbName, _ := os.LookupEnv("SHIORI_PG_NAME")
sslmode, ok := os.LookupEnv("SHIORI_PG_SSLMODE")
Copy link
Member

Choose a reason for hiding this comment

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

Keep in mind that in this case ok will be true in the case SHIORI_PG_SSLMODE is an empty string. Unsure how Posgres will react to that. Maybe is better to check for sslMode == "".

@ancientcatz
Copy link

When will this PR be merged into the master?

@fmartingr
Copy link
Member

When will this PR be merged into the master?

Waiting for this comment to be addressed.

@fmartingr
Copy link
Member

Thank you for your contribution!

@fmartingr fmartingr merged commit e02d3e7 into go-shiori:master Jul 9, 2023
@fmartingr fmartingr added this to the 1.5.5 milestone Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants