Write-DbaDbTableData: Fix for connections to Azure SQL Database with AccessToken - Githubissues

dataplat / dbatools

🚀 SQL Server automation and instance migrations have never been safer, faster or freer

https://dbatools.io

MIT License

2.39k stars 787 forks source link

Write-DbaDbTableData: Fix for connections to Azure SQL Database with AccessToken #9401

Closed andreasjordan closed 1 day ago

andreasjordan commented 1 week ago

Type of Change

[x] Bug fix (non-breaking change, fixes #9394 )
[ ] New feature (non-breaking change, adds functionality, fixes # )
[ ] Breaking change (affects multiple commands or functionality, fixes # )
[ ] Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
[ ] Adding code coverage to existing functionality
[ ] Pester test is included
[ ] If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
[ ] Unit test is included
[ ] Documentation
[ ] Build system

See issue for details.

Also included in this pull request:

Import-DbaCsv is now also supported, as I removed the all the StatementTimeout 0 as they are the default anyway.
Added a note in the documentation of AccessToken in Connect-DbaInstance on which commands are supported.

andreasjordan commented 1 week ago

Import-DbaCsv has the same problems - because it uses Connect-DbaInstance with -StatementTimeout 0. That forces a .ConnectionContext.Copy() and that does not work with a token.

Will try to find a good solution for all of those commands.

andreasjordan commented 1 week ago

No, it's $connContext.GetDatabaseConnection($Database) that is the problem. Will have to test if this is needed...

andreasjordan commented 1 week ago

OK, I have to do more tests. Will do this in the next days...

andreasjordan commented 1 week ago

Not .GetDatabaseConnection($Database) is the problem, but .ConnectionContext.Copy(). This creates a new connection which fails in case the original connection was opened using AccessToken.

Bottom line: $server from $server = Connect-DbaInstance ... -AccessToken must not be changed by any Connect-DbaInstance inside of our commands.

potatoqualitee commented 3 days ago

Thank you, will take some time to review this one

potatoqualitee commented 3 days ago

Well it wont take a long time, just will check it out more deeply tomorrow 😅

potatoqualitee commented 1 day ago

im happy to merge this but how will we handle timeouts? i imagine the timeouts were adding for expected long-running queries. should we advise updating their configs?

potatoqualitee commented 1 day ago

And thank you 🙏🏼

andreasjordan commented 1 day ago

As StatementTimeout 0 was and is the default, there are no timeouts. So nothing was changed an that side and there should not be a problem.