Skip to content

Commit

Permalink
Adopt flyteidl's ordered variable map change (#158)
Browse files Browse the repository at this point in the history 
* Adopt flyteidl's ordered variable map change

Signed-off-by: Sean Lin <sean@union.ai>

* wip Adopt flyteidl's ordered variable map change

Signed-off-by: Sean Lin <sean@union.ai>

* wip Adopt flyteidl's ordered variable map change

Signed-off-by: Sean Lin <sean@union.ai>

* Reformat json

Signed-off-by: Sean Lin <sean@union.ai>

* Reformat json

Signed-off-by: Sean Lin <sean@union.ai>

* Parameter field name change

Signed-off-by: Sean Lin <sean@union.ai>

* Make execfile backward compatible

Signed-off-by: Sean Lin <sean@union.ai>

* Remove legacy proto

Signed-off-by: Sean Lin <sean@union.ai>

* Format test data

Signed-off-by: Sean Lin <sean@union.ai>

* Update flyteidl

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* goimports

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Co-authored-by: Haytham Abuelfutuh <haytham@afutuh.com>
  • Loading branch information
@mayitbeegh @EngHabu
mayitbeegh and EngHabu committed Sep 2, 2021
1 parent dcc2c09 commit 7c31c1e
Show file tree
Hide file tree
Showing 20 changed files with 1,221 additions and 645 deletions.
52 changes: 30 additions & 22 deletions flytectl/cmd/create/execution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ func createExecutionSetup() {
},
},
}
variableMap := map[string]*core.Variable{
"sorted_list1": &sortedListLiteralType,
"sorted_list2": &sortedListLiteralType,
variableMap := []*core.VariableMapEntry{
{
Name: "sorted_list1",
Var: &sortedListLiteralType,
}, {
Name: "sorted_list2",
Var: &sortedListLiteralType,
},
}

task1 := &admin.Task{
Expand All @@ -56,9 +61,10 @@ func createExecutionSetup() {
},
}
mockClient.OnGetTaskMatch(ctx, mock.Anything).Return(task1, nil)
parameterMap := map[string]*core.Parameter{
"numbers": {
Var: &core.Variable{
parameterMap := []*core.ParameterMapEntry{
{
Name: "numbers",
Parameter: &core.Parameter{Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_CollectionType{
CollectionType: &core.LiteralType{
Expand All @@ -68,40 +74,42 @@ func createExecutionSetup() {
},
},
},
},
}},
},
"numbers_count": {
Var: &core.Variable{
{
Name: "numbers_count",
Parameter: &core.Parameter{Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_INTEGER,
},
},
},
}},
},
"run_local_at_count": {
Var: &core.Variable{
{
Name: "run_local_at_count",
Parameter: &core.Parameter{Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_INTEGER,
},
},
},
Behavior: &core.Parameter_Default{
Default: &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Primitive{
Primitive: &core.Primitive{
Value: &core.Primitive_Integer{
Integer: 10,
Behavior: &core.Parameter_Default{
Default: &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Primitive{
Primitive: &core.Primitive{
Value: &core.Primitive_Integer{
Integer: 10,
},
},
},
},
},
},
},
},
}},
},
}
launchPlan1 := &admin.LaunchPlan{
Expand Down
22 changes: 11 additions & 11 deletions flytectl/cmd/create/serialization_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import (
// MakeLiteralForVariables builds a map of literals for the provided serialized values. If a provided value does not have
// a corresponding variable or if that variable is invalid (e.g. doesn't have Type property populated), it returns an
// error.
func MakeLiteralForVariables(serialize map[string]interface{}, variables map[string]*core.Variable) (map[string]*core.Literal, error) {
func MakeLiteralForVariables(serialize map[string]interface{}, variables []*core.VariableMapEntry) (map[string]*core.Literal, error) {
types := make(map[string]*core.LiteralType)
for k, v := range variables {
t := v.GetType()
for _, e := range variables {
t := e.GetVar().GetType()
if t == nil {
return nil, fmt.Errorf("variable [%v] has nil type", k)
return nil, fmt.Errorf("variable [%v] has nil type", e.GetName())
}

types[k] = t
types[e.GetName()] = t
}

return MakeLiteralForTypes(serialize, types)
Expand All @@ -28,15 +28,15 @@ func MakeLiteralForVariables(serialize map[string]interface{}, variables map[str
// MakeLiteralForParams builds a map of literals for the provided serialized values. If a provided value does not have
// a corresponding parameter or if that parameter is invalid (e.g. doesn't have Type property populated), it returns an
// error.
func MakeLiteralForParams(serialize map[string]interface{}, parameters map[string]*core.Parameter) (map[string]*core.Literal, error) {
func MakeLiteralForParams(serialize map[string]interface{}, parameters []*core.ParameterMapEntry) (map[string]*core.Literal, error) {
types := make(map[string]*core.LiteralType)
for k, v := range parameters {
if variable := v.GetVar(); variable == nil {
return nil, fmt.Errorf("parameter [%v] has nil Variable", k)
for _, e := range parameters {
if variable := e.GetParameter().GetVar(); variable == nil {
return nil, fmt.Errorf("parameter [%v] has nil Variable", e.GetName())
} else if t := variable.GetType(); t == nil {
return nil, fmt.Errorf("parameter [%v] has nil variable type", k)
return nil, fmt.Errorf("parameter [%v] has nil variable type", e.GetName())
} else {
types[k] = t
types[e.GetName()] = t
}
}

Expand Down
52 changes: 33 additions & 19 deletions flytectl/cmd/create/serialization_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,16 @@ func TestMakeLiteralForParams(t *testing.T) {
}

t.Run("Happy path", func(t *testing.T) {
inputParams := map[string]*core.Parameter{
"a": {
Var: &core.Variable{
inputParams := []*core.ParameterMapEntry{
{
Name: "a",
Parameter: &core.Parameter{Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_STRING,
},
},
},
}},
},
}

Expand All @@ -85,18 +86,22 @@ func TestMakeLiteralForParams(t *testing.T) {
})

t.Run("Invalid Param", func(t *testing.T) {
inputParams := map[string]*core.Parameter{
"a": nil,
inputParams := []*core.ParameterMapEntry{
{
Name: "a",
Parameter: nil,
},
}

_, err := MakeLiteralForParams(inputValues, inputParams)
assert.Error(t, err)
})

t.Run("Invalid Type", func(t *testing.T) {
inputParams := map[string]*core.Parameter{
"a": {
Var: &core.Variable{},
inputParams := []*core.ParameterMapEntry{
{
Name: "a",
Parameter: &core.Parameter{Var: &core.Variable{}},
},
}

Expand All @@ -111,11 +116,14 @@ func TestMakeLiteralForVariables(t *testing.T) {
}

t.Run("Happy path", func(t *testing.T) {
inputVariables := map[string]*core.Variable{
"a": {
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_STRING,
inputVariables := []*core.VariableMapEntry{
{
Name: "a",
Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_STRING,
},
},
},
},
Expand All @@ -127,18 +135,24 @@ func TestMakeLiteralForVariables(t *testing.T) {
})

t.Run("Invalid Variable", func(t *testing.T) {
inputVariables := map[string]*core.Variable{
"a": nil,
inputVariables := []*core.VariableMapEntry{
{
Name: "a",
Var: nil,
},
}

_, err := MakeLiteralForVariables(inputValues, inputVariables)
assert.Error(t, err)
})

t.Run("Invalid Type", func(t *testing.T) {
inputVariables := map[string]*core.Variable{
"a": {
Type: nil,
inputVariables := []*core.VariableMapEntry{
{
Name: "a",
Var: &core.Variable{
Type: nil,
},
},
}

Expand Down
34 changes: 17 additions & 17 deletions flytectl/cmd/get/execution_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func CreateAndWriteExecConfigForWorkflow(wlp *admin.LaunchPlan, fileName string)
return WriteExecConfigToFile(executionConfig, fileName)
}

func TaskInputs(task *admin.Task) map[string]*core.Variable {
taskInputs := map[string]*core.Variable{}
func TaskInputs(task *admin.Task) []*core.VariableMapEntry {
taskInputs := []*core.VariableMapEntry{}
if task == nil || task.Closure == nil {
return taskInputs
}
Expand All @@ -81,22 +81,22 @@ func TaskInputs(task *admin.Task) map[string]*core.Variable {
func ParamMapForTask(task *admin.Task) (map[string]yaml.Node, error) {
taskInputs := TaskInputs(task)
paramMap := make(map[string]yaml.Node, len(taskInputs))
for k, v := range taskInputs {
varTypeValue, err := coreutils.MakeDefaultLiteralForType(v.Type)
for _, e := range taskInputs {
varTypeValue, err := coreutils.MakeDefaultLiteralForType(e.Var.Type)
if err != nil {
fmt.Println("error creating default value for literal type ", v.Type)
fmt.Println("error creating default value for literal type ", e.Var.Type)
return nil, err
}
var nativeLiteral interface{}
if nativeLiteral, err = coreutils.ExtractFromLiteral(varTypeValue); err != nil {
return nil, err
}

if k == v.Description {
if e.Name == e.Var.Description {
// a: # a isn't very helpful
paramMap[k], err = getCommentedYamlNode(nativeLiteral, "")
paramMap[e.Name], err = getCommentedYamlNode(nativeLiteral, "")
} else {
paramMap[k], err = getCommentedYamlNode(nativeLiteral, v.Description)
paramMap[e.Name], err = getCommentedYamlNode(nativeLiteral, e.Var.Description)
}
if err != nil {
return nil, err
Expand All @@ -105,8 +105,8 @@ func ParamMapForTask(task *admin.Task) (map[string]yaml.Node, error) {
return paramMap, nil
}

func WorkflowParams(lp *admin.LaunchPlan) map[string]*core.Parameter {
workflowParams := map[string]*core.Parameter{}
func WorkflowParams(lp *admin.LaunchPlan) []*core.ParameterMapEntry {
workflowParams := []*core.ParameterMapEntry{}
if lp == nil || lp.Spec == nil {
return workflowParams
}
Expand All @@ -119,27 +119,27 @@ func WorkflowParams(lp *admin.LaunchPlan) map[string]*core.Parameter {
func ParamMapForWorkflow(lp *admin.LaunchPlan) (map[string]yaml.Node, error) {
workflowParams := WorkflowParams(lp)
paramMap := make(map[string]yaml.Node, len(workflowParams))
for k, v := range workflowParams {
varTypeValue, err := coreutils.MakeDefaultLiteralForType(v.Var.Type)
for _, e := range workflowParams {
varTypeValue, err := coreutils.MakeDefaultLiteralForType(e.Parameter.Var.Type)
if err != nil {
fmt.Println("error creating default value for literal type ", v.Var.Type)
fmt.Println("error creating default value for literal type ", e.Parameter.Var.Type)
return nil, err
}
var nativeLiteral interface{}
if nativeLiteral, err = coreutils.ExtractFromLiteral(varTypeValue); err != nil {
return nil, err
}
// Override if there is a default value
if paramsDefault, ok := v.Behavior.(*core.Parameter_Default); ok {
if paramsDefault, ok := e.Parameter.Behavior.(*core.Parameter_Default); ok {
if nativeLiteral, err = coreutils.ExtractFromLiteral(paramsDefault.Default); err != nil {
return nil, err
}
}
if k == v.Var.Description {
if e.Name == e.Parameter.Var.Description {
// a: # a isn't very helpful
paramMap[k], err = getCommentedYamlNode(nativeLiteral, "")
paramMap[e.Name], err = getCommentedYamlNode(nativeLiteral, "")
} else {
paramMap[k], err = getCommentedYamlNode(nativeLiteral, v.Var.Description)
paramMap[e.Name], err = getCommentedYamlNode(nativeLiteral, e.Parameter.Var.Description)
}

if err != nil {
Expand Down
13 changes: 9 additions & 4 deletions flytectl/cmd/get/execution_util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func TestTaskInputs(t *testing.T) {
taskInputs := map[string]*core.Variable{}
taskInputs := []*core.VariableMapEntry{}
t.Run("nil task", func(t *testing.T) {
retValue := TaskInputs(nil)
assert.Equal(t, taskInputs, retValue)
Expand Down Expand Up @@ -60,9 +60,14 @@ func createTask() *admin.Task {
},
}

variableMap := map[string]*core.Variable{
"sorted_list1": &sortedListLiteralType,
"sorted_list2": &sortedListLiteralType,
variableMap := []*core.VariableMapEntry{
{
Name: "sorted_list1",
Var: &sortedListLiteralType,
}, {
Name: "sorted_list2",
Var: &sortedListLiteralType,
},
}

inputs := &core.VariableMap{
Expand Down
4 changes: 2 additions & 2 deletions flytectl/cmd/get/launch_plan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ var launchplanColumns = []printer.Column{
{Header: "Type", JSONPath: "$.closure.compiledTask.template.type"},
{Header: "State", JSONPath: "$.spec.state"},
{Header: "Schedule", JSONPath: "$.spec.entityMetadata.schedule"},
{Header: "Inputs", JSONPath: "$.closure.expectedInputs.parameters." + printer.DefaultFormattedDescriptionsKey + ".var.description"},
{Header: "Outputs", JSONPath: "$.closure.expectedOutputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"},
{Header: "Inputs", JSONPath: "$.closure.expectedInputs.parameters[0].parameter.var.description"},
{Header: "Outputs", JSONPath: "$.closure.expectedOutputs.variables[0].var.description"},
}

// Column structure for get all launchplans
Expand Down
Loading

0 comments on commit 7c31c1e

Please sign in to comment.