Fix | FailoverPartner key check in SqlConnectionPoolGroupProviderInfo #1614
Conversation
…ts value from the SqlConnectionString
lcheunglci
changed the title
|
Wonder if a test is possible - why does this crop up? |
|
I think it depends on how the Sql Server is configured, so a special test agent/vm might be required in the build/test pipeline to be able to verify this properly. |
| @@ -104,7 +104,8 @@ private PermissionSet CreateFailoverPermission(SqlConnectionString userConnectio | |||
| // the server, we will use that name over what was specified | |||
| // in the original connection string. | |||
|
|
|||
| if (null == userConnectionOptions[SqlConnectionString.KEY.FailoverPartner]) | |||
| if (userConnectionOptions.ContainsKey(SqlConnectionString.KEY.FailoverPartner) && | |||
There was a problem hiding this comment.
I'm curious how a connection without AG setup can reach this line! I tried locally (Win 10) and in Win Server 2019, but neither got to this line.
Could you reproduce it?
There was a problem hiding this comment.
I don't think the issue occurs without a AG setup. I tried locally prior to this and wasn't able to reproduce.
There was a problem hiding this comment.
I have just experienced this issue in a non-AG setup.
|
/azp run CI-SqlClient |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
|
No pipelines are associated with this pull request. |
…verPartnerKeyMissing
Codecov Report
@@ Coverage Diff @@
## main #1614 +/- ##
==========================================
- Coverage 71.54% 70.80% -0.75%
==========================================
Files 291 293 +2
Lines 61241 64256 +3015
==========================================
+ Hits 43817 45494 +1677
- Misses 17424 18762 +1338
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Is this getting patched into v4? |
Relates to issue #1594 and #1613 , I think we can either do a check if the key exists first before retrieving the value from the SqlConnectionString using the Dictionary get operator[] or use TryGetValue with null as the default value if that's the expected behaviour because I don't think it's obvious that we need to specify
FailOverPartnerset it to an empty string.