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

Added MSSQL domain auth example. #11799

Merged
merged 1 commit into from Jan 17, 2020
Merged

Added MSSQL domain auth example. #11799

merged 1 commit into from Jan 17, 2020

Conversation

engineertdog
Copy link
Contributor

@engineertdog engineertdog commented Jan 7, 2020

Creating this pull request to add an example to the MSSQL section for domain authentication. It took me a while to figure out how to do so in Sequelize when it was working in pure Tedious. The line in the docs, shown below, is what caused the confusion. Due to the statement, I had thought that all MSSQL specific configuration settings had to be in options under dialectOptions. Maybe the text can be changed, but I've added an example nonetheless.

Please note: tedious@^6.0.0 requires you to nest MSSQL specific options inside an additional options-object inside the dialectOptions-object.

@codecov
Copy link

codecov bot commented Jan 17, 2020

Codecov Report

Merging #11799 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11799   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files          94       94           
  Lines        9191     9191           
=======================================
  Hits         8848     8848           
  Misses        343      343

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bd2be8...fc82890. Read the comment docs.

@papb
Copy link
Member

papb commented Jan 17, 2020

Hello! I see you are a first-time contributor, thank you for taking the time to help Sequelize! I hope to see more PRs from you in the future! 😬

@papb
Copy link
Member

papb commented Jan 17, 2020

I have rebased your PR manually since my recently-merged PR #11825 caused a conflict here.

papb
papb approved these changes Jan 17, 2020
@engineertdog
Copy link
Contributor Author

engineertdog commented Jan 17, 2020

Ah, no problem. There doesn't seem to be many people working with MSSQL, so it took me a while to figure out that the documentation wasn't exactly as clear as it could have been when it came to domain auth. The textual representation was a bid misleading in the docs. But, looks good!

@papb papb merged commit eb9d232 into sequelize:master Jan 17, 2020
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants