tedious-connection-pool
tedious-connection-pool copied to clipboard
Updating tedious Dependency to 2.1.1
Description
The tedious dependency has been updated from 1.14.0 to 2.1.1. Unit tests have been modified slightly as they would fail when running against sql server hosted as a container locally.
Related Issue
#43
Motivation and Context
Although this pull request addresses an existing issue to update the tedious pkg, I personally wanted to leverage the new streaming capabilities added since [email protected].
How Has This Been Tested?
This change has not affected existing tests. No new tests have been added. I used a docker container running mssql to runt the tests against.
docker run -e 'ACCEPT_EULA=Y' -e 'MSSQL_SA_PASSWORD=Password12!' -p 1433:1433 --name dev -d microsoft/mssql-server-linux:2017-latest
I also ran the following scripts to set up the test environment in sql server
CREATE DATABASE test
CREATE LOGIN test WITH PASSWORD=N'test', DEFAULT_DATABASE=test, CHECK_POLICY=OFF
GRANT ALTER ANY CONNECTION TO test
USE test
CREATE USER test FOR LOGIN test WITH DEFAULT_SCHEMA=dbo
ALTER ROLE db_owner ADD MEMBER test
USE msdb
CREATE USER test FOR LOGIN test WITH DEFAULT_SCHEMA=dbo
ALTER ROLE SQLAgentOperatorRole ADD MEMBER test
ALTER ROLE SQLAgentReaderRole ADD MEMBER test
ALTER ROLE SQLAgentUserRole ADD MEMBER test
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [x] All new and existing tests passed.
@arthurschreiber @v-suhame
Can you help to get this PR merged? It looks very reasonable.
Without this change, users of the tedious-connection-pool
package get the old Tedious version 1.15.0.
@chdh I'll try and get this merged soon, although I'm not 100% sure about the release process around this package. 👍
@arthurschreiber Any updates?
@kibertoad as I’m Still on travel until Saturday, I can’t really do much here. Also, upgrading to tedious@2
is a breaking change, as it dropped support for older NodeJS versions and will require a major version bump on this the connection pool library as well.
I’ll try to get to this as soon as I can. 🙇🏻♂️
@arthurschreiber Would it be useful if I also created a PR that would drop pre-6 Node support from this library as well?
@arthurschreiber thanks for following up! I just wanted to check in one more time- if this isn't getting published, no worries, will publish this fork as a scoped package for the time being
Any reason why this has not been merged?