Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

Basic version of create segment using azure open ai and semantic kernel sdk #6

Merged
merged 15 commits into from
Aug 11, 2023

Conversation

rosireddyr
Copy link
Collaborator

@rosireddyr rosireddyr commented Aug 10, 2023

Motivation and Context

This pr contains 3 basic examples of create segments.
Its having create segment skill and make it work with example file.

Description

Contribution Checklist

@@ -25,58 +25,58 @@ public static async Task Main()
await Example_00_03_OpenApiSkill_PlayFab.RunAsync().SafeWaitAsync(cancelToken);

// Run examples
await Example01_NativeFunctions.RunAsync().SafeWaitAsync(cancelToken);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can ignore changes to this file for the PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will revert. But all the examples will run whenever we run the project, so these are not required and save time if we comment.

.WithAzureTextCompletionService("text-davinci-003", TestConfiguration.AzureOpenAI.Endpoint, TestConfiguration.AzureOpenAI.ApiKey) // Note: Action Planner works with old models like text-davinci-002
.Build();

string folder = RepoFiles.SampleSkillsPath();
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string folder = RepoFiles.SampleSkillsPath();

why do you need this line?
Remove if possible #Resolved

Console.WriteLine("======== Action Planner ========");
var kernel2 = new KernelBuilder()
.WithLogger(ConsoleLogger.Logger)
.WithAzureTextCompletionService("text-davinci-003", TestConfiguration.AzureOpenAI.Endpoint, TestConfiguration.AzureOpenAI.ApiKey) // Note: Action Planner works with old models like text-davinci-002
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"text-davinci-003"

Use the model name from the Test Configuration (not hard-coded) #Resolved

// Create a segment skill
{
Console.WriteLine("======== Action Planner ========");
var kernel2 = new KernelBuilder()
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kernel2

rename to just 'kernel' instead of kernel2 #Resolved

private static async Task CreateSegmentExample(string goal)
{
// Create a segment skill
{
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  { [](http://example.com/codeflow?start=1&length=8)

remove brackets #Resolved

"Create a segment with name WelcomeEgyptNewPlayers for the players located in the Egypt with entered segment action of email notification?", // With entered segment action
"Create a segment with name EgyptNewPlayers for the players located in the Egypt?" // If the segment already exist, create a segment with name appended with guid
};
await CreateSegmentExample(goals[0]);
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await CreateSegmentExample(goals[0]);

loop the questions #Resolved

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these are create operations, same segment name won't work every time for now. Later I will add logic to rename if the segment already exist.

/// </summary>
public sealed class SegmentSkill
{
ContextVariables contextVariables = new ContextVariables();
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ContextVariables contextVariables = new ContextVariables();

remove from here and move into the skil function #Resolved

{
//ToDo: Create payload json using Playfab dlls/sdk
// Set properties to create a Segment using swagger.json
ContextVariables contextVariables = new ContextVariables();
Copy link
Collaborator

@nir-schleyen nir-schleyen Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new ContextVariables();

just new()
(as you did in line 48) #Resolved

countryCodes.Add("Australia", "AU");
countryCodes.Add("Kenya", "KE");
countryCodes.Add("Egypt", "EG");
countryCodes.Add("China", "CN");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this shouldn't be needed. With an appropriate description, the model should return an appropriate country codes. Country codes are well known and the model should just have the right documentation to tell it to do so

Copy link
Collaborator Author

@rosireddyr rosireddyr Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wav.. It worked just by updating the description :)

string segmentPayload = "{\n \"SegmentModel\": {\n \"Name\": \"<SegmentName>\",\n \"SegmentOrDefinitions\": [\n {\n \"SegmentAndDefinitions\": [\n {\n \"<SegmentDefinition>\": {\n \"LogInDate\": \"<SegmentComparisonValue>T00:00:00Z\",\n \"Comparison\": \"<SegmentComparison>\"\n }\n }\n ]\n }\n ]\n }\n }";
string locationPayload = "{\n \"SegmentModel\": {\n \"Name\": \"<SegmentName>\",\n \"SegmentOrDefinitions\": [\n {\n \"SegmentAndDefinitions\": [\n {\n \"<SegmentDefinition>\": {\n \"CountryCode\": \"<SegmentComparisonValue>\",\n \"Comparison\": \"<SegmentComparison>\"\n }\n }\n ]\n }\n ]\n }\n }";

if (segmentdefinition == "LocationFilter")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocationFilter

With proper documentation this mapping steps won't be needed, and the model should know to return country codes (instead of country names) for that segment definition

Copy link
Collaborator Author

@rosireddyr rosireddyr Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wav.. It worked just by updating description :)

ContextVariables contextVariables = new();
contextVariables.Set("content_type", "application/json");
contextVariables.Set("server_url", TestConfiguration.PlayFab.Endpoint);
string segmentPayload = GetSegmentPayload(segmentname, segmentdefinition, segmentcomparison, segmentcomparisonvalue, segmentaction, segmentactioncode, segmentactionvalue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetSegmentPayload

Can you try having the model to generate the proper segment payload so you don't have to do it yourself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I will look into it after this pr. Just want to keep some working code.

Copy link
Collaborator

@nir-schleyen nir-schleyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@rosireddyr rosireddyr merged commit 464cf38 into main Aug 11, 2023
6 of 16 checks passed
@rosireddyr rosireddyr deleted the RR_Segment branch August 11, 2023 17:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants