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

Ideas #16

Closed
Silex opened this issue Mar 15, 2021 · 3 comments
Closed

Ideas #16

Silex opened this issue Mar 15, 2021 · 3 comments

Comments

@Silex
Copy link
Contributor

Silex commented Mar 15, 2021

Hello!

  1. Add an ability to detect if we are already connected, right now Get-ManagementServer throw and it's impractical.
  2. Add a -Force parameter to Connect-ManagementServer, right now it just warns but stays connected to the first server. Also I'd argue that it should be an error instead of a warning 😉

The idea for 1 & 2 is to simplify the following script:

# Bad, if DoStuff throws we never disconnect, then when we run again with another server the first one gets used!
Connect-ManagementServer -Server $server
DoStuff
Disconnect-ManagementServer

# Better
try {
  Connect-ManagementServer -Server $server
  DoStuff
}
finally {
  try { Disconnect-ManagementServer } catch {}
}

# The best solution IMHO, we don't really care if we don't disconnect as long as we are sure the connect does the right thing
Connect-ManagementServer -Server $server -Force
DoStuff
Disconnect-ManagementServer -Force

These kind of problems can bite you hard if you fool around in ISE and change $server and run and boom, you just modified the previously connected server.

And about my previous reports about -ErrorAction not being respected and $ErrorActionPreference working etc, it's all explained here MicrosoftDocs/PowerShell-Docs#1583 (with a more pragmatic approach explained here https://stackoverflow.com/questions/9294949/when-should-i-use-write-error-vs-throw-terminating-vs-non-terminating-errors/39949027#39949027)

Basically powershell is a real mess when it comes to errors.

@joshooaj
Copy link
Collaborator

joshooaj commented May 6, 2021

For the next release (possibly being pushed up to PSGallery tomorrow) I've made a number of changes including...

  • Connect-ManagementServer now has a Force switch so Disconnect-ManagementServer will be called silently if needed before connecting to the provided management server
  • Connect-ManagementServer without the Force switch will throw a terminating error if you're already connected to a management server (totally agree it's risky to just throw a warning there!)
  • Disconnect-ManagementServer now returns silently if you're not already connected so you can safely call Disconnect whether you're connected or not.
  • Get-ManagementServer writes an error instead of a terminating error if you're not connected to a management server. So you can more easily use Get-ManagementServer to test whether you're connected right now. Use it in a try/catch with ErrorAction Stop or test whether Get-ManagementServer -ErrorAction Ignore returns null
  • An internal change in the way we keep track of the site we logged into makes it possible to use MilestonePSTools with runspaces to parallelize operations without having to use Connect-ManagementServer in each runspace/thread. Previously you might get an obnoxious "You're not connected" error even though it seems like you should have been. It doesn't help with PSJobs because those run in separate powershell.exe processes but runspaces running in the same process can now operate easily against the milestone connection established in the "parent thread".

Thanks for the feedback and apologies for the delay!

@joshooaj
Copy link
Collaborator

joshooaj commented May 7, 2021

Thanks again - closing this now with the release of MilestonePSTools 1.1.0 on PSGallery

@joshooaj joshooaj closed this as completed May 7, 2021
@Silex
Copy link
Contributor Author

Silex commented May 8, 2021

Fantastic! Thanks.

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

No branches or pull requests

2 participants