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

Add ImportPSModulesFromPath support #178

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

beargle
Copy link

@beargle beargle commented Mar 7, 2018

Add Start-RSJob parameter ModulesFromPathToImport that accepts a single path, from which module(s) will be imported into background runspace jobs. Modules in this path are not required to be listed by name, and the path does not have to exist in $env:PSModulePath. Fixes #177.

Changes proposed in this pull request:

  • Import module(s) into runspace job from a path specified by Start-RSJob parameter -ModulesFromPathToImport

How to test this code:
The snippet below allows manual verification of Get-Module output when either a full or relative module path name is passed. I'm relatively new to Pester, but would like to learn if someone has suggestions on how to setup the automated test cases.

# assuming PoshRSJob is in $env:PSModulePath or imported, setup a sample custom module path.
# get both a full and relative path name for testing
$ModulePath = New-Item -Path ([Guid]::NewGuid().ToString()) -ItemType Directory
$RelativeModulePath = Resolve-Path -Path $ModulePath -Relative

# export sample modules for import in runspace
svn export https://github.com/psake/psake/tags/v4.7.0/src $ModulePath\psake | Out-Null;
svn export https://github.com/ramblingcookiemonster/PSDeploy/trunk/PSDeploy $ModulePath\PSDeploy | Out-Null;

# start single runspace job, passing full name of module path.
# Get-Module output in the runspace should show imported psake and PSDeploy modules.
"Modules from full path" | Start-RSJob -Name { $_ } -ModulesFromPathToImport $ModulePath -ScriptBlock {
    Get-Module;
} | Wait-RSJob | Receive-RSJob;

# do it again, this time with a relative module path
"Modules from relative path" | Start-RSJob -Name { $_ } -ModulesFromPathToImport $RelativeModulePath -Verbose -ScriptBlock {
    Get-Module;
} | Wait-RSJob | Receive-RSJob;

# cleanup
Remove-Item -Recurse -Force $ModulePath;
Remove-Variable -Name @("ModulePath", "RelativeModulePath");

Has been tested on (remove any that don't apply):

  • PowerShell 4.0
  • Windows 7 and 8.1

beargle added 3 commits March 5, 2018 22:31
Add parameter `ModulesFromPathToImport` that accepts a single path, from
which module(s) will be imported into background runspace jobs. Modules
in this path are not required to be listed by name, and the path does
not have to exist in `$env:PSModulePath`.
When the '-ModulesFromPathToImport' parameter is passed for
'Start-RSJob', it should handle either a full and relative path name.

Also raise an exception if the module(s) path is not found, which means
none of the module(s) are imported in the runspace job. An exception
prevents the runspace jobs from starting, as any dependent functions
referenced inside the 'ScriptBlock' parameter would not be found.
`Resolve-Path` fires a non-terminating error when it can't find a
specified path, which means the error isn't caught for handling
in a Catch block. Make `Resolve-Path` raise a terminating error,
so we can re-throw to add context to the original exception message.
@proxb proxb self-requested a review March 7, 2018 21:06
@proxb proxb self-assigned this Mar 7, 2018
@proxb proxb removed their request for review March 7, 2018 21:07
@proxb
Copy link
Owner

proxb commented Mar 21, 2018

@beargle Any chance you could look at using the method shown here: #177 (comment)? It would be nice to keep all module importing down to a single parameter if possible. If it doesn't work out though, I can move forward to accept this PR.

@proxb proxb mentioned this pull request Mar 21, 2018
@beargle
Copy link
Author

beargle commented Mar 22, 2018

@proxb Sure, I saw the comment, but am not sold on readability of pre-processing module paths before passing to Start-RSJob.

It seems more intuitive to be able to pass a single "root" module path, so that everything (properly organized as importable, of course) gets loaded into the runspace jobs.

I totally get the usability issue with having two parameters that could be used for module import...do you think it would be confusing to wrap functionality of both InitialSessionState.ImportPSModule and InitialSessionState.ImportPSModulesFromPath into the single -ModulesToImport parameter?

@proxb
Copy link
Owner

proxb commented Mar 23, 2018

do you think it would be confusing to wrap functionality of both InitialSessionState.ImportPSModule and InitialSessionState.ImportPSModulesFromPath into the single -ModulesToImport parameter?

@beargle I think that would be a better way to go at least to support both the module name and module path requirement even though it will require a little more work to handle both of the possible values being supplied to the parameter. While I get the ease of just supplying a single root math containing all of the modules, I'm just not sure about having multiple module import parameters.

Remove separate '-ModulesFromPathToImport' parameter, adding this
functionality to existing '-ModulesToImport' parameters. This means
'-ModulesToImport' can accept a collection containing:

 - Installed module names
 - Path (full or relative) containing one or more modules
 - Path (full or relative) to a module manifest, script module, or
   binary module file

Purpose of consolidation is to simplify module import for the caller;
They should have the flexibility to pass a root module path for
recursive import, or be able to specify modules similar to `Import-Module`,
with minimal preprocessing of the parameter collection.
@beargle
Copy link
Author

beargle commented Mar 24, 2018

@proxb Makes sense, I've consolidated the module path import into existing -ModulesToImport parameter, and updated the original PR text. It should accept these different ways of specifying modules for import:

  • Installed module names
Start-RSJob -ModulesToImport "PSDeploy"...
  • Path (full or relative) to a module manifest, script module, or binary module file
Start-RSJob -ModulesToImport ".\PSDeploy\PSDeploy.psd1"...
  • Path (full or relative) containing one or more modules
Start-RSJob -ModulesToImport ".\workspace\Modules"...
  • Combination of any of the above
Start-RSJob -ModulesToImport "PSDeploy", ".\workspace\Modules"...

Let me know your thoughts, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants