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

fix Function on Kubernetes for Python V2 progamming model #3676

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);

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

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);

Choose a reason for hiding this comment

The 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"},

Choose a reason for hiding this comment

The 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" } } }
});

Choose a reason for hiding this comment

The 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
}

Choose a reason for hiding this comment

The 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
Expand Up @@ -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
{
Expand Down Expand Up @@ -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)
{
Expand All @@ -220,7 +226,9 @@ private async Task<TriggersPayload> GetTriggersLocalFiles()
}
}

var functionsJsons = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated
var useMetadata = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated || IsWorkerIndexingEnabled();
Copy link
Contributor

Choose a reason for hiding this comment

The 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);

Expand All @@ -233,6 +241,19 @@ private async Task<TriggersPayload> GetTriggersLocalFiles()
};
}

private static bool IsWorkerIndexingEnabled()
Copy link
Contributor

Choose a reason for hiding this comment

The 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");
Expand Down