-
Notifications
You must be signed in to change notification settings - Fork 467
Connection String in the Config #851
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
Comments
@rubenvde can you tell me what connection strings you've attempted to use that don't work? |
I'm going to close this as I think that there is no bug here and I'd like to know what configuration you're using that means there's "no way" to do what you want. Note that from our connection string parser (https://github.com/tediousjs/node-mssql/blob/v5.0.5/lib/connectionstring.js#L225) you can set |
I checked your latest test files and found the solution by adding a setting to the connectionstring like so in the unit.js. Thanks! |
I know this is "closed", but almost 5 years later this gap still exists and I don't understand why the "manually modify the connection string" solution was acceptable to the authors or the original requester. In the environment I'm working in, we have a connection string that is passed in through a container's environment. This connection string, from the perspective of the application, is "opaque". It may already contain a "default" request timeout value (used by all the "normal" connections in the code). In a small corner of the code meant to handle long-running operations for populating a cache, we want to override just the request timeout. As far as I can tell, this library still does not support this without manually parsing the connection string: your parser doesn't seem to be exported, and we can't pass in overrides to the |
hi @mccolljr, thanks for the feedback. I'm not entirely sure I agree with your logic about the opaque nature of a connection string. If you're able to make a decision about wanting to set a "longer" request timeout in a small corner of your code, that corner of the code needs to be able to know (or assume) that the connection string's timeout is not long enough as it is - a decision to discard the potentially defined connection string value is still required. What if your small corner sets a request timeout that is shorter than the connection string had defined? The opaqueness of the connection string is irrelevant if you're going make domain specific decisions about the settings needed in certain circumstances. Whilst I accept that I could see a use case for overriding specific settings with a supplementary configuration object, the main reason this doesn't exist is because no one has wanted this feature enough to submit an enhancement to the library for it. In terms of no better solutions... there are two.
|
I can understand the general point, but in our case it is a matter of "the default might be up to a minute, but we know our query needs 5-8 minutes". We have knowledge of the time required by our cache filling queries, and we want to simply configure our private connection pool with a timeout that we know is sufficient, regardless of what might already be in the connection string (or not).
Ah, this would be perfect. Thank you for pointing this out to me, I looked for a top-level function and did not consider it would be scoped under
If the connection string can be parsed into a bag of options using the method you mentioned above, I think maybe it is just a matter of mentioning this in the documentation of the |
Expected behaviour:
I want to use the
await sql.connect("mssql://")
function but change the default requestTimeout in the config file.Actual behaviour:
There is no way to actually doing this. The config json doesn't accept connection strings.
Configuration:
Just a string starting with "mssql://"
Software versions
The text was updated successfully, but these errors were encountered: