Skip to content

Refactor the code base to make it easy to support concurrency within a worker in future #117

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

Merged
merged 2 commits into from
Dec 17, 2018

Conversation

daxian-dbw
Copy link
Contributor

@daxian-dbw daxian-dbw commented Dec 15, 2018

Refactor the code base to make it easy to support concurrency within a worker in future.

  • Add PowerShellManagerPool , but it's just the skeleton. Today the pool only has one PowerShellManager instance. We can add the real pool implementation if we decide to support concurrency within a worker process.
  • Setup the PSModule environment variable as part of Runspace.Open by using InitialSessionState.EnvironmentVariables. This fixes the mysterious ModulePathShouldBeSetCorrectly test failure by making sure the env variable is set to the expected one as part of the Runspace.Open of every PowerShellManager. It failed before because:
    1. dotnet test runs tests in parallel by default
    2. when creating a new PowerShellManager, the PSModulePath environment variable will be set again within Runspace.Open (by SetModulePath called from ModuleIntrisic constructor). That makes value unexpected.
  • Update tests to make them more reliable.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

This looks great Dongbo 😃

I just had a couple questions

@daxian-dbw daxian-dbw merged commit bf4ba5a into dev Dec 17, 2018
@daxian-dbw daxian-dbw deleted the pool branch December 17, 2018 08:58
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.

2 participants