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

propagating ssl=true from --url into dialectOptions #788

Closed
wants to merge 1 commit into from

Conversation

igorski89
Copy link

When connecting to the database using the --url option I noticed the ssl options are ignored. The only option being to revert to a config file specifying dialectOptions directly there.

While it worked just fine it felt too cumbersome to manage when the rest of the app configuration is made via env variables.

Digging a little bit into the code turns out the url parser fully supports the ssl=true option yet doesn't propagates it further into the config object to have a Sequelize instance initialised.

This small change addresses the issue.

Fixes #154

@ahimta
Copy link

ahimta commented Jul 9, 2019

This is especially important because you will most likely have all users with a require-ssl option and wouldn't be able to connect otherwise:sweat_smile:.

However, the bigger issue maybe that many stuff in the URL are not passed and not just SSL:sweat_smile:.

@mannol
Copy link

mannol commented Aug 31, 2019

This would be a really great feature to have. It seems straight forward enough for merge, @sushantdhiman?

@dmayle
Copy link

dmayle commented Jun 1, 2020

@mannol @sushantdhiman @papb This actually fixes a real issue, any chance it can still be merged?

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.

How do I run the migration with SSL enabled?
5 participants