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

node-mysql2 dialect not supporting password usage #341

Closed
schwenx opened this Issue Jun 26, 2014 · 2 comments

Comments

Projects
None yet
2 participants
@schwenx
Copy link

schwenx commented Jun 26, 2014

When trying to connect to a MySQL database by using the newly added node-mysql2 dialect, it is not possible to connect to a database using a password.

Reason: I researched a bit myself in the repository and have seen that the used connection method within Knex.js' mysql2 dialect is using 'user', 'database' and 'connection' property only:

(...\knex\lib\dialects\mysql2\index.js)
var connection = mysql2.createConnection(_.pick(this.connectionSettings, 'user', 'database', 'connection'));

Is there any reason for doing so? According to the node-mysql2 description and docs it should be fully API compatible to the node-mysql module, so I'd suggest to just use same .createConnection() call as it is used in Knex' mysql dialect:

var connection = mysql.createConnection(this.connectionSettings); 

I tried it out and now it also did work when using a password for database access...

Many greets, and thanks for the amazing module,
Stefan.

@tgriesser

This comment has been minimized.

Copy link
Owner

tgriesser commented Jun 26, 2014

Yeah I have no idea why that's in there. Will fix.

@tgriesser

This comment has been minimized.

Copy link
Owner

tgriesser commented Jun 27, 2014

It looked like the reason was that if invalid options were passed, node-mysql2 really didn't seem to like it. I went ahead and picked out all of the options that seemed relevant password, ssl, connection, stream - pushed in the latest release, let me know if you have any more issues.

@tgriesser tgriesser closed this Jun 27, 2014

@tgriesser tgriesser added the fixed label Jun 27, 2014

elliotf pushed a commit to elliotf/knex that referenced this issue Nov 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.