fix Function on Kubernetes for Python V2 progamming model #3676
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
|
@vrdmr / @gavin-aguiar Could you review this? |
ping @vrdmr @gavin-aguiar - thank you! |
| @@ -233,6 +241,19 @@ private async Task<TriggersPayload> GetTriggersLocalFiles() | |||
| }; | |||
| } | |||
|
|
|||
| private static bool IsWorkerIndexingEnabled() | |||
There was a problem hiding this comment.
This is not needed. Worker indexing is default now
| @@ -220,7 +226,9 @@ private async Task<TriggersPayload> GetTriggersLocalFiles() | |||
| } | |||
| } | |||
|
|
|||
| var functionsJsons = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated | |||
| var useMetadata = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated || IsWorkerIndexingEnabled(); | |||
There was a problem hiding this comment.
Why are we checking for dotnet isolated?
| { | ||
| var functionAppPath = Path.Combine(functionsPath, "function_app.py"); | ||
|
|
||
| var functionAppContents = await File.ReadAllTextAsync(functionAppPath); |
There was a problem hiding this comment.
Wrap I/O operations in try-catch blocks and log errors
| { | ||
| {"type", $"httpTrigger"}, | ||
| {"direction", "In"}, | ||
| {"name", "req"}, |
There was a problem hiding this comment.
Lots of repeated constants can go on top of the file for readability or a method can we created to better manage bindings for different triggerType..
| private static List<Dictionary<string, object>> ParsePythonFunctionApp(string contents) | ||
| { | ||
| var functionMetadataList = new List<Dictionary<string, object>>(); | ||
| var functionPattern = new Regex(@"@app\.(\w+)(?:_trigger)?\(([^)]+)\)\s+def (\w+)\(", RegexOptions.Compiled | RegexOptions.Multiline); |
There was a problem hiding this comment.
NIT: Add a comment explaining the what this regex does.
| {"connection", properties["connection"]}, | ||
| {"cardinality", "One"}, | ||
| {"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
| }); |
There was a problem hiding this comment.
Checking: should you flatten the nested dictionary with an class (or an entity)?
| }); | ||
| break; | ||
| // Add additional cases for other trigger types if necessary | ||
| } |
There was a problem hiding this comment.
If a new or unknown trigger type is encountered, it silently does nothing. This can cause hard-to-debug issues. IMHO extra logging will help here.
Issue describing the changes in this PR
Resolves python programming V2 does not work with function on kubernetes
Pull request checklist