-
Notifications
You must be signed in to change notification settings - Fork 444
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
fix Function on Kubernetes for Python V2 progamming model #3676
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
using Newtonsoft.Json; | ||
using Newtonsoft.Json.Linq; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Text.RegularExpressions; | ||
using System.Threading.Tasks; | ||
|
||
public class FunctionMetadataGenerator | ||
{ | ||
|
||
public static async Task GenerateFunctionsMetadata(string functionsPath) | ||
{ | ||
var functionAppPath = Path.Combine(functionsPath, "function_app.py"); | ||
|
||
var functionAppContents = await File.ReadAllTextAsync(functionAppPath); | ||
var functionMetadataList = ParsePythonFunctionApp(functionAppContents); | ||
var functionsMetadataJson = JsonConvert.SerializeObject(functionMetadataList, Formatting.Indented); | ||
var functionsMetadataPath = Path.Combine(functionsPath, "functions.metadata"); | ||
await File.WriteAllTextAsync(functionsMetadataPath, functionsMetadataJson); | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Add a comment explaining the what this regex does. |
||
var matches = functionPattern.Matches(contents); | ||
|
||
foreach (Match match in matches) | ||
{ | ||
var triggerType = match.Groups[1].Value; | ||
var triggerProperties = match.Groups[2].Value; | ||
var functionName = match.Groups[3].Value; | ||
|
||
var properties = ParseTriggerProperties(triggerProperties); | ||
|
||
var bindings = new List<Dictionary<string, object>>(); | ||
|
||
switch (triggerType) | ||
{ | ||
case "route": | ||
var authLevel = properties.GetValueOrDefault("auth_level", "Function") | ||
.Split('.') | ||
.LastOrDefault(); | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", $"httpTrigger"}, | ||
{"direction", "In"}, | ||
{"name", "req"}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.. |
||
{"authLevel", authLevel}, | ||
{"methods", new[] {"get", "post"}}, // Assuming default methods are get and post | ||
}); | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "http"}, | ||
{"direction", "Out"}, | ||
{"name", "$return"} | ||
}); | ||
break; | ||
case "timer_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", $"timerTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "myTimer")}, | ||
{"schedule", properties["schedule"]} | ||
}); | ||
break; | ||
case "blob_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", $"blobTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "stream")}, | ||
{"path", properties["path"]}, | ||
{"connection", properties["connection"]}, | ||
{"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
}); | ||
break; | ||
case "cosmos_db_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "cosmosDBTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "input")}, | ||
{"databaseName", properties["database_name"]}, | ||
{"containerName", properties["container_name"]}, | ||
{"connection", properties["connection"]}, | ||
{"leaseContainerName", "leases"}, | ||
{"createLeaseContainerIfNotExists", true} | ||
}); | ||
break; | ||
case "event_hub_message_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "eventHubTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "events")}, | ||
{"eventHubName", properties["event_hub_name"]}, | ||
{"connection", properties["connection"]}, | ||
{"cardinality", "Many"}, | ||
{"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
}); | ||
break; | ||
case "queue_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "queueTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "message")}, | ||
{"queueName", properties["queue_name"]}, | ||
{"connection", properties["connection"]}, | ||
{"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
}); | ||
break; | ||
case "service_bus_queue_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "serviceBusTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "message")}, | ||
{"queueName", properties["queue_name"]}, | ||
{"connection", properties["connection"]}, | ||
{"cardinality", "One"}, | ||
{"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
}); | ||
break; | ||
case "service_bus_topic_trigger": | ||
bindings.Add(new Dictionary<string, object> | ||
{ | ||
{"type", "serviceBusTrigger"}, | ||
{"direction", "In"}, | ||
{"name", properties.GetValueOrDefault("arg_name", "message")}, | ||
{"topicName", properties["topic_name"]}, | ||
{"subscriptionName", properties["subscription_name"]}, | ||
{"connection", properties["connection"]}, | ||
{"cardinality", "One"}, | ||
{"properties", new Dictionary<string, object> { { "supportsDeferredBinding", "True" } } } | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
functionMetadataList.Add(new Dictionary<string, object> | ||
{ | ||
{"name", functionName}, | ||
{"bindings", bindings} | ||
}); | ||
} | ||
|
||
return functionMetadataList; | ||
} | ||
|
||
|
||
private static Dictionary<string, string> ParseTriggerProperties(string properties) | ||
{ | ||
var propertiesDict = new Dictionary<string, string>(); | ||
var propPattern = new Regex(@"(\w+)=(?:""([^""]*)""|'([^']*)'|([^,\s]+))", RegexOptions.Compiled); | ||
var propMatches = propPattern.Matches(properties); | ||
|
||
foreach (Match propMatch in propMatches) | ||
{ | ||
var key = propMatch.Groups[1].Value; | ||
var value = propMatch.Groups[2].Success ? propMatch.Groups[2].Value : | ||
propMatch.Groups[3].Success ? propMatch.Groups[3].Value : | ||
propMatch.Groups[4].Value; | ||
propertiesDict[key] = value; | ||
} | ||
|
||
return propertiesDict; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,7 +121,8 @@ public override async Task RunAsync() | |
await DockerHelpers.DockerBuild(resolvedImageName, Environment.CurrentDirectory); | ||
} | ||
// This needs to be fixed to run after the build. | ||
triggers = await DockerHelpers.GetTriggersFromDockerImage(resolvedImageName); | ||
//triggers = await DockerHelpers.GetTriggersFromDockerImage(resolvedImageName); | ||
triggers = await GetTriggersLocalFiles(); | ||
} | ||
else | ||
{ | ||
|
@@ -209,6 +210,11 @@ public override async Task RunAsync() | |
private async Task<TriggersPayload> GetTriggersLocalFiles() | ||
{ | ||
var functionsPath = Environment.CurrentDirectory; | ||
var pythonFunctionFile = Path.Combine(functionsPath, "function_app.py"); | ||
if (FileSystemHelpers.FileExists(pythonFunctionFile)) | ||
{ | ||
await FunctionMetadataGenerator.GenerateFunctionsMetadata(functionsPath); | ||
} | ||
if (GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnet || | ||
GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated) | ||
{ | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we checking for dotnet isolated? |
||
|
||
var functionsJsons = useMetadata | ||
? ReadFunctionsMetadata(functionsPath) | ||
: ReadFunctionJsons(functionsPath); | ||
|
||
|
@@ -233,6 +241,19 @@ private async Task<TriggersPayload> GetTriggersLocalFiles() | |
}; | ||
} | ||
|
||
private static bool IsWorkerIndexingEnabled() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed. Worker indexing is default now |
||
{ | ||
var localSettingsPath = Path.Combine(Environment.CurrentDirectory, "local.settings.json"); | ||
if (FileSystemHelpers.FileExists(localSettingsPath)) | ||
{ | ||
var localSettingsContents = FileSystemHelpers.ReadAllTextFromFile(localSettingsPath); | ||
var localSettingsJson = JsonConvert.DeserializeObject<JObject>(localSettingsContents); | ||
var featureFlags = localSettingsJson["Values"]?["AzureWebJobsFeatureFlags"]?.ToString(); | ||
return featureFlags?.Contains("EnableWorkerIndexing") ?? false; | ||
} | ||
return false; | ||
} | ||
|
||
private static Dictionary<string, JObject> ReadFunctionsMetadata(string functionsPath) | ||
{ | ||
var functionsMetadataPath = Path.Combine(functionsPath, "functions.metadata"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap I/O operations in try-catch blocks and log errors