Skip to content
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

Refactoring of Copy-DbaCredential and Export-DbaCredential #8129

Merged
merged 2 commits into from Feb 4, 2022

Conversation

andreasjordan
Copy link
Contributor

General refactoring to be able to open DAC inside of Copy-DbaCredential.
Fixed some issues with QUOTENAME of credential name

Still open: Opening DAC in Export like in Copy. Will have to save all the $server variables to be able to disconnect at the end.


$copyCredentialStatus = [pscustomobject]@{
SourceServer = $sourceServer.Name
DestinationServer = $destServer.Name
SourceServer = $sourceServer.DomainInstanceName
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ComputerName instead? I feel like there are sometimes problems with DomainInstanceName. Perhaps it's not always filled out or accurate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SourceServer is used for the output object in all the Copy commands @potatoqualitee

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you are referring to property name? If that, yes we use ComputerName for the value in other commands.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for double checking 😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks like Connect object does checks for populating ComputerName

if ($server.DatabaseEngineType -ne "SqlAzureDatabase") {
try {
Write-Message -Level Debug -Message "we try to use server.NetName for computername"
$computername = $server.NetName
} catch {
Write-Message -Level Debug -Message "Ups, failed so we use instance.ComputerName"
$computername = $instance.ComputerName
}
Write-Message -Level Debug -Message "Ok, computername = server.NetName = '$computername'"
}
# SQL on Linux is often on docker and the internal name is not useful
if (-not $computername -or $server.HostPlatform -eq "Linux") {
Write-Message -Level Debug -Message "SQL on Linux is often on docker and the internal name is not useful - we use instance.ComputerName as computername"
$computername = $instance.ComputerName
Write-Message -Level Debug -Message "Ok, computername is now '$computername'"
}
Write-Message -Level Debug -Message "We add IsAzure = '$false', ComputerName = computername = '$computername', DbaInstanceName = instance.InstanceName = '$($instance.InstanceName)', NetPort = instance.Port = '$($instance.Port)', ConnectedAs = server.ConnectionContext.TrueLogin = '$($server.ConnectionContext.TrueLogin)'"
Add-Member -InputObject $server -NotePropertyName IsAzure -NotePropertyValue $false -Force
Add-Member -InputObject $server -NotePropertyName ComputerName -NotePropertyValue $computername -Force
Add-Member -InputObject $server -NotePropertyName DbaInstanceName -NotePropertyValue $instance.InstanceName -Force

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't need the computername here, but "SqlInstance". This is the output from the currently released code:

image

And we mostly fill "SqlInstance" using ".DomainInstanceName", just search for "SqlInstance = $server.DomainInstanceName" in the code.

We can not use ".Name" of "$server", as this has the prefix "ADMIN:" when using a DAC - and I think we don't want to have that in the output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhhh make sense, thanks

@potatoqualitee potatoqualitee merged commit 89b8aa0 into development Feb 4, 2022
@potatoqualitee potatoqualitee deleted the CopyDbaCredential_refactor branch February 4, 2022 09:12
@potatoqualitee
Copy link
Member

Thank you 🙇🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants