Skip to content

[Revamp pipeline thread handling] Restore dialogue about PackageManagement installation #1576

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

Closed
rjmholt opened this issue Sep 30, 2021 · 6 comments · Fixed by #1735
Closed
Assignees
Milestone

Comments

@rjmholt
Copy link
Contributor

rjmholt commented Sep 30, 2021

From #1459 (comment).

The ExecutePSCommandAsync() API should allow us to both print errors to the console and throw them to the caller so they can take appropriate action.

Then we can use a try/catch at the callsite to handle the behaviour accordingly.

@ghost ghost added the Needs: Triage Maintainer attention needed! label Sep 30, 2021
@andyleejordan andyleejordan removed the Needs: Triage Maintainer attention needed! label Oct 5, 2021
@andyleejordan andyleejordan added this to the Committed milestone Jan 12, 2022
@andyleejordan andyleejordan moved this to Todo in Sea Biscuit Jan 20, 2022
@andyleejordan
Copy link
Member

@SydneyhSmith We need to decide if now is the time to remove all our workflows around package management, as it may not be the extension's place to deal with it.

@SydneyhSmith
Copy link
Collaborator

@andschwa I cant see anywhere PackageManagement is used by the extension so I am okay with not adding this back, and waiting for customer asks for it (reading back through old issues and PRs I am just seeing issues with it)...that being said I am totally open to the possibility that I am missing an important reason why this exists and would be happy to schedule time to add it back if that reason rises to the surface...

@SeeminglyScience
Copy link
Collaborator

Sorry I didn't see this. It's less to do with our workflows and more due to an old version of PackageManagement was causing dead locks due to some of the async stuff they were doing.

See #762

That old version may no longer cause the dead lock though, since we are no longer dependent on PowerShell's event system. Needs testing to be sure though.

@SydneyhSmith
Copy link
Collaborator

Thanks @SeeminglyScience that's super helpful

@SeeminglyScience
Copy link
Collaborator

SeeminglyScience commented Mar 1, 2022

Yeah I think we can strip all of this out now. Went into Windows PowerShell, reverted to stock PackageManagement and PowerShellGet, and intellisense no longer dead locks. Then if I go back to stable and I do Find-Package -<tab> it immediately dead locks.

This makes sense since we're no longer using the OnIdle event, so PackageManagement is now able to successfully marshal back to the pipeline thread. Side note, that does also mean PowerShell eventing is working in some respect.

This is also good news because it means anyone who has been ignoring our prompt will now magically get way more stable 🎉

SeeminglyScience added a commit to SeeminglyScience/PowerShellEditorServices that referenced this issue Mar 1, 2022
@andyleejordan
Copy link
Member

Heck yeah!

andyleejordan pushed a commit that referenced this issue Mar 10, 2022
The reason we attempted to update `PackageManagement` is no longer applicable
(the deadlock has been fixed) so now we don't have to won this logic.

Fixes #1576 and #1721.
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 a pull request may close this issue.

4 participants