msdsn package should support azuresql:// driver name #768
Conversation
|
Sorry, pushed prematurely. Fixing. |
Codecov Report
@@ Coverage Diff @@
## master #768 +/- ##
==========================================
- Coverage 71.32% 71.21% -0.12%
==========================================
Files 24 24
Lines 5388 5388
==========================================
- Hits 3843 3837 -6
- Misses 1301 1305 +4
- Partials 244 246 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
dagss
force-pushed
the
fixup-msdsn+azuresql
branch
from
3bd6584
to
f49b1da
Compare
|
Force-pushed proper fix + test coverage |
There was a problem hiding this comment.
I don't know why you need to have a azuresql:// prefix. I don't understand this need. I don't think we would want to add Schema in Config. I would be okay with returning a "schema" value within the map[string]string return value, that would be set within splitConnectionStringURL.
| @@ -117,7 +118,7 @@ func Parse(dsn string) (Config, map[string]string, error) { | |||
| return p, params, err | |||
| } | |||
| params = parameters | |||
| } else if strings.HasPrefix(dsn, "sqlserver://") { | |||
| } else if strings.HasPrefix(dsn, "sqlserver://") || strings.HasPrefix(dsn, "azuresql://") { | |||
There was a problem hiding this comment.
If we do this, let's create a global regexp:
^[a-zA-Z]+:\/\/
To test for this.
| @@ -127,6 +128,11 @@ func Parse(dsn string) (Config, map[string]string, error) { | |||
| params = splitConnectionString(dsn) | |||
| } | |||
|
|
|||
| p.Scheme = "sqlserver" | |||
There was a problem hiding this comment.
This would not go here.
|
The azuresql:// prefix is not my idea and I found it inconvenient... it is part of the "azuread" subpackage that is already merged and I just followed the lead from there. I would much prefer if the AD options were possible to pass on a standard sqlserver:// DSN |
|
@dagss the separate azuread package was designed to avoid pulling in azure dependencies for apps that didn't want to use AAD ever. |
Attempting to switch to the new AD connector in
azureadpackage instead of our in-house homegrown connector. When usingmsdsn.Parsewithazuresql://it parses the string the wrong way.