Skip to content

Commit 4b74dbb

Browse files
author
Anthony Staples
committed
Add explanatory comments
1 parent dd4716a commit 4b74dbb

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

src/RequestProcessor.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
namespace Microsoft.Azure.Functions.PowerShellWorker
2020
{
2121
using Microsoft.Azure.Functions.PowerShellWorker.WorkerIndexing;
22+
using Microsoft.PowerShell;
2223
using System.Diagnostics;
2324
using System.Text.Json;
2425
using LogLevel = Microsoft.Azure.WebJobs.Script.Grpc.Messages.RpcLog.Types.Level;
@@ -124,6 +125,10 @@ internal StreamingMessage ProcessWorkerInitRequest(StreamingMessage request)
124125
RemoteSessionNamedPipeServer.CreateCustomNamedPipeServer(pipeName);
125126
}
126127

128+
// Previously, this half of the dependency management would happen just prior to the dependency download in the
129+
// first function load request. Now that we have the FunctionAppDirectory in the WorkerInitRequest,
130+
// we can do the setup of these variables in the function load request. We need these variables initialized
131+
// for the FunctionMetadataRequest, should it be sent.
127132
try
128133
{
129134
var rpcLogger = new RpcLogger(_msgStream);
@@ -136,8 +141,8 @@ internal StreamingMessage ProcessWorkerInitRequest(StreamingMessage request)
136141
rpcLogger.Log(isUserOnlyLog: false, LogLevel.Trace, string.Format(PowerShellWorkerStrings.FirstFunctionLoadCompleted, stopwatch.ElapsedMilliseconds));
137142
}
138143
catch (Exception e)
139-
{
140-
// Failure that happens during this step is terminating and we will need to return a failure response to
144+
{
145+
// This is a terminating failure: we will need to return a failure response to
141146
// all subsequent 'FunctionLoadRequest'. Cache the exception so we can reuse it in future calls.
142147
_initTerminatingError = e;
143148

@@ -231,7 +236,7 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request)
231236
}
232237
catch (Exception e)
233238
{
234-
// Failure that happens during this step is terminating and we will need to return a failure response to
239+
// This is a terminating failure: we will need to return a failure response to
235240
// all subsequent 'FunctionLoadRequest'. Cache the exception so we can reuse it in future calls.
236241
_initTerminatingError = e;
237242

@@ -366,7 +371,6 @@ internal StreamingMessage ProcessInvocationCancelRequest(StreamingMessage reques
366371

367372
private StreamingMessage ProcessFunctionMetadataRequest(StreamingMessage request)
368373
{
369-
// WorkerStatusResponse type says that it is not used but this will create an empty one anyway to return to the host
370374
StreamingMessage response = NewStreamingMessageTemplate(
371375
request.RequestId,
372376
StreamingMessage.ContentOneofCase.FunctionMetadataResponse,

src/WorkerIndexing/WorkerIndexingHelper.cs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,44 @@ internal static IEnumerable<RpcFunctionMetadata> IndexFunctions(string baseDir)
1919
{
2020
List<RpcFunctionMetadata> indexedFunctions = new List<RpcFunctionMetadata>();
2121

22+
// This is not the correct way to deal with getting a runspace for the cmdlet.
23+
24+
// Firstly, creating a runspace is expensive. If we are going to generate a runspace, it should be done on
25+
// the function load request so that it can be created while the host is processing.
26+
27+
// Secondly, this assumes that the AzureFunctions.PowerShell.SDK module is present on the machine/VM's
28+
// PSModulePath. On an Azure instance, it will not be. What we need to do here is move the call
29+
// to SetupAppRootPathAndModulePath in RequestProcessor to the init request, and then use the
30+
// _firstPwshInstance to invoke the Get-FunctionsMetadata command. The only issue with this is that
31+
// SetupAppRootPathAndModulePath needs the initial function init request in order to know if managed
32+
// dependencies are enabled in this function app.
33+
34+
// Proposed solutions:
35+
// 1. Pass ManagedDependencyEnabled flag in the worker init request
36+
// 2. Change the flow, so that _firstPwshInstance is initialized in worker init with the PSModulePath
37+
// assuming that managed dependencies are enabled, and then revert the PSModulePath in the first function
38+
// init request should the managed dependencies not be enabled.
39+
// 3. Continue using a new runspace for invoking Get-FunctionsMetadata, but initialize it in worker init and
40+
// point the PsModulePath to the module path bundled with the worker.
41+
42+
2243
InitialSessionState initial = InitialSessionState.CreateDefault();
2344
Runspace runspace = RunspaceFactory.CreateRunspace(initial);
2445
runspace.Open();
25-
System.Management.Automation.PowerShell ps = System.Management.Automation.PowerShell.Create();
26-
ps.Runspace = runspace;
46+
System.Management.Automation.PowerShell _powershell = System.Management.Automation.PowerShell.Create();
47+
_powershell.Runspace = runspace;
2748

28-
ps.AddCommand(GetFunctionsMetadataCmdletName).AddArgument(baseDir);
49+
_powershell.AddCommand(GetFunctionsMetadataCmdletName).AddArgument(baseDir);
2950
string outputString = string.Empty;
30-
foreach (PSObject rawMetadata in ps.Invoke())
51+
foreach (PSObject rawMetadata in _powershell.Invoke())
3152
{
3253
if (outputString != string.Empty)
3354
{
3455
throw new Exception(PowerShellWorkerStrings.GetFunctionsMetadataMultipleResultsError);
3556
}
3657
outputString = rawMetadata.ToString();
3758
}
38-
ps.Commands.Clear();
59+
_powershell.Commands.Clear();
3960

4061
List<FunctionInformation> functionInformations = JsonConvert.DeserializeObject<List<FunctionInformation>>(outputString);
4162

0 commit comments

Comments
 (0)