Add MSSQL DB client #37
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sylvestre67
force-pushed
the
syl/mssql-client
branch
from
8e8537a to
2e93ada
Compare
Sylvestre67
force-pushed
the
syl/mssql-client
branch
from
295a9ff to
2e93ada
Compare
Sylvestre67
commented
Oct 26, 2022
added 25 commits
October 26, 2022 15:53
Sylvestre67
force-pushed
the
syl/mssql-client
branch
from
3923b35 to
6710555
Compare
|
|
||
| if (!validate(body)) throw badRequest(); | ||
|
|
||
| res.setHeader("Content-Type", "text/plain"); |
There was a problem hiding this comment.
This doesn't seem like the right content-type? I can't remember if there is a reason for this.
There was a problem hiding this comment.
added 7 commits
October 27, 2022 14:38
Sylvestre67
force-pushed
the
syl/mssql-client
branch
from
6a2fdf0 to
721fc8b
Compare
Sylvestre67
commented
Nov 1, 2022
Sylvestre67
commented
Nov 1, 2022
|
This won't actually work for people unless the other PR with the UI changes is deployed, and that PR won't work until this is deployed. We could just merge this and tell people not to use it, or merge it and not make a new release for it until we merge the client, or just merge when the other PR is ready. |
|
Approved. This all looks good to me, but I think we should loop Visnu in on how best to merge it. |
wiltsecarpenter
approved these changes
Nov 3, 2022
|
|
||
| if (!validate(body)) throw badRequest(); | ||
|
|
||
| res.setHeader("Content-Type", "text/plain"); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves Microsoft Server SQL DB client
Description
Adds a database client for MS SQL databases. The client is set up so it can be seamlessly imported in the data-connector project.
Besides the main client, it also adds the basic to write mocha integration-test and the
docker-composesetup to run the test from alocalenvironment andCI.Review notes
The core of the changes can be found in the new
lib/mssql.jsfile. It defines thecheckandquerymethod, andconnection pool.The
routinghas been adjusted to match the one exposed indata-connector.Tests
To run the test locally with
Docker, runTest will:
mssqlinstance, using theazure-sql-edgedocker imagetest_mssql.bakbackup file and create another read only userTo run the test as in
CIwithDocker, runTest will:
mssqlinstance, using theserver:2019-latestdocker imagetest_mssql.bakbackup file and create another read only userNOTE: The
citests won't run on aMacBook PRO M1environment. Run on aFedora Linuxdistribution successfully.Risk
[If something goes wrong, how big of a problem is it? Consider only the impact, not the probability.]
[😅 Low - We'll need to add a PR to roll this back, but it won't cause problems for users]