Skip to content

remove -computer parameter from get-process #4960

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.

Sign up for GitHub

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

Jump to bottom
Merged
adityapatwardhan merged 1 commit into from
Oct 18, 2017
Merged

remove -computer parameter from get-process #4960

adityapatwardhan merged 1 commit into from
Oct 18, 2017

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Sep 30, 2017

Due to bug https://github.com/dotnet/corefx/issues/24357 [Process]::GetProcesses(computer) returns local processes. Since this previously relied on dotnet remoting, I don't think we should support this and users should use PSRP instead.

@SteveL-MSFT SteveL-MSFT added Breaking-Change breaking change that may affect users Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Sep 30, 2017
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 1, 2017 •
edited
Loading

Although -ComputerName works with Get-Service, we should consider removing it from Get-Service, Remove-Service, and Set-Service so that users consistently rely on PSRP and only having that port open

@joeyaiello
Copy link
Contributor

joeyaiello commented Oct 4, 2017

We don't have quorum with the @PowerShell/powershell-committee today, but I'm formally acking that I'm okay with this one.

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 5, 2017

I'm continuing to use WMI/RPC in a corporate environment.
This is familiar and convenient, as well as meet existing corporate security settings - changing it requires too much expense.
Before removing this I'd prefer to get support -PSSession in all the cmdlets.

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 5, 2017

@iSazonov corefx has this in their 2.1.0 milestone so it won't be fixed by the time we get to final. If PSRP is allowed in your environment, you can always use Invoke-Command to run it remotely. I agree that long term, having an automatic -PSSession parameter would be best.

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 5, 2017

@SteveL-MSFT Does it make sense to do automatic -PSSession parameter in 6.1.0 milestone?

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 5, 2017

It's certainly something we could look at. Perhaps you could open an new issue to track it.

@iSazonov iSazonov mentioned this pull request Oct 8, 2017
Closed
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 12, 2017

@PowerShell/powershell-committee reviewed this and agreed to remove the -ComputerName parameter from this and the *-Service cmdlets. We may reuse that parameter in the future for remoting over PSRP so it was best to remove it now in 6.0.0.

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Oct 12, 2017
@iSazonov
Copy link
Collaborator

iSazonov commented Oct 12, 2017

Should we be consistency and remove the parameter from all cmslets forcing users to use PSRemoting?

PS C:\Windows\system32> gcm -ParameterName ComputerName

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Cmdlet          Add-Computer                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Clear-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Connect-PSSession                                  3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Enter-PSSession                                    3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Get-Counter                                        3.0.0.0    Microsoft.PowerShell.Diagnostics
Cmdlet          Get-EventLog                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-HotFix                                         3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-Process                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-PSSession                                      3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Get-Service                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-WinEvent                                       3.0.0.0    Microsoft.PowerShell.Diagnostics
Cmdlet          Get-WmiObject                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Invoke-Command                                     3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Invoke-WmiMethod                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Limit-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          New-EventLog                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          New-PSSession                                      3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Receive-Job                                        3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Receive-PSSession                                  3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Register-WmiEvent                                  3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-Computer                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-EventLog                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-PSSession                                   3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Remove-WmiObject                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Rename-Computer                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Restart-Computer                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Send-MailMessage                                   3.1.0.0    Microsoft.PowerShell.Utility
Cmdlet          Set-Service                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Set-WmiInstance                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Show-EventLog                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Stop-Computer                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Test-Connection                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Write-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 12, 2017

@iSazonov it looks like you enumerated the Windows PowerShell cmdlets, but yes, we should remove it from the other cmdlets except the ones specific to remoting.

@SteveL-MSFT SteveL-MSFT mentioned this pull request Oct 12, 2017
Closed
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 12, 2017

@iSazonov I created #5090 as I think only the *-Service ones need it removed, the remaining ones are ok.

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 12, 2017

@anmenaga since this was approved by Committee to proceed, can you review?

anmenaga
anmenaga approved these changes Oct 12, 2017
View reviewed changes
Copy link

@anmenaga anmenaga left a comment

Choose a reason for hiding this comment

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

Change looks ok;
But looking that there is no corresponding test change, it feels strange that there is no existing test for "get-process -computer".
Maybe it is a good idea to run feature tests on this change to be sure that tests won't break...

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 12, 2017

@anmenaga pretty sure this isn't being used in our tests at all since I found this via code coverage analysis, but will run feature tests anyways

adityapatwardhan
adityapatwardhan approved these changes Oct 12, 2017
View reviewed changes
Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

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

LGTM

@SteveL-MSFT
[feature]
151d078 
remove -computer support since corefx Process.GetProcesses(computer) returns local processes
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Oct 18, 2017

@adityapatwardhan can we merge?

@adityapatwardhan adityapatwardhan merged commit 847846d into PowerShell:master Oct 18, 2017
@SteveL-MSFT SteveL-MSFT deleted the process-computer branch November 12, 2017 05:17
This was referenced Dec 5, 2017
Merged
Merged
@matt9ucci matt9ucci mentioned this pull request Feb 28, 2018
Merged
8 tasks
@iSazonov iSazonov mentioned this pull request Mar 5, 2020
Closed
@rjmholt rjmholt mentioned this pull request Apr 15, 2020
Closed
xtqqczze added a commit to xtqqczze/PowerShell-PowerShell that referenced this pull request Mar 17, 2021
@xtqqczze
Remove unnecessary Array -> List -> Array conversion
8316bed 
Change removes redundant code from ProcessBaseCommand.AllProcesse that was left after existing functionality was removed in PowerShell#4960.
@xtqqczze xtqqczze mentioned this pull request Mar 17, 2021
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

+1 more reviewer

@anmenaga anmenaga anmenaga approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan adityapatwardhan

Labels

Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@SteveL-MSFT @joeyaiello @iSazonov @anmenaga @adityapatwardhan