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

PS Core adjustments related to Test-Connection #4870

Merged
merged 6 commits into from
Dec 31, 2018

Conversation

nvarscar
Copy link
Contributor

@nvarscar nvarscar commented Dec 21, 2018

Let's see how many commands will blow after this one

Type of Change

  • Bug fix (non-breaking change, fixes Resolve-DbaNetworName is not working properly in PS6.0+ #4840)
  • New feature (non-breaking change, adds functionality)
  • Breaking change (effects multiple commands or functionality)
  • 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/sqlcollaborative/appveyor-lab ?
  • Nunit test is included
  • Documentation
  • Build system

Purpose

Resolving issues in the commands relying on Test-Connection function in PS6 and a few other issues I discovered while testing them on PS Core

Approach

Resolve-DbaNetworkName was rewritten, reducing the number of calls to DNS.
Test-Connection was replaced by

$ping = New-Object System.Net.NetworkInformation.Ping
$timeout = 1000 #milliseconds
$reply = $ping.Send($computer, $timeout)

Commands to test

Resolve-DbaNetworkName

Screenshots

Learning

Test-Connection is a different beast in PS6.0 and has different set of properties
$PSVersionTable.CLRVersion is null in Core.

@nvarscar
Copy link
Contributor Author

Phew! We're finally in the green. Please review at your leisure 😅

@potatoqualitee
Copy link
Member

thanks much, Kirill! @niphlod you around to test with your specific network?

@potatoqualitee
Copy link
Member

so far, im not getting the same results in my own testing

dev:
image

this branch:

image

@nvarscar
Copy link
Contributor Author

Woops. Never tested it with websites, gonna look into it.

@potatoqualitee
Copy link
Member

i was trying to "impersonate" niph's dual dns environment and couldn't think of anything else 😅

@potatoqualitee
Copy link
Member

also, merry christmas, my friend 🤗🎄

@nvarscar
Copy link
Contributor Author

I've updated some parts of the code to better build the output object. The difference in ComputerName however is (I think) is a flow in current code logic, since it gets that name right now from the input string unchanged due to being unable to connect via PS remoting. The turbo mode returns proper Computer name from DNS resolution.
New version of code uses DNS name resolution in all cases (which kinda makes sense, to me, at least) and only updates it again when remoting works.

@nvarscar
Copy link
Contributor Author

And Merry Christmas!

@potatoqualitee
Copy link
Member

thank you both 🙇

@potatoqualitee potatoqualitee merged commit 4428dbc into dataplat:development Dec 31, 2018
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.

Resolve-DbaNetworName is not working properly in PS6.0+
3 participants