Skip to content

Commit

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

This reverts commit 7c31c1e.
Signed-off-by: Sean Lin <[email protected]>
  • Loading branch information
@mayitbeegh
mayitbeegh authored Sep 10, 2021
1 parent 2761a72 commit 8db28dd
Show file tree
Hide file tree
Showing 20 changed files with 645 additions and 1,221 deletions.
52 changes: 22 additions & 30 deletions flytectl/cmd/create/execution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,9 @@ func createExecutionSetup() {
},
},
}
variableMap := []*core.VariableMapEntry{
{
Name: "sorted_list1",
Var: &sortedListLiteralType,
}, {
Name: "sorted_list2",
Var: &sortedListLiteralType,
},
variableMap := map[string]*core.Variable{
"sorted_list1": &sortedListLiteralType,
"sorted_list2": &sortedListLiteralType,
}

task1 := &admin.Task{
Expand All @@ -61,10 +56,9 @@ func createExecutionSetup() {
},
}
mockClient.OnGetTaskMatch(ctx, mock.Anything).Return(task1, nil)
parameterMap := []*core.ParameterMapEntry{
{
Name: "numbers",
Parameter: &core.Parameter{Var: &core.Variable{
parameterMap := map[string]*core.Parameter{
"numbers": {
Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_CollectionType{
CollectionType: &core.LiteralType{
Expand All @@ -74,42 +68,40 @@ func createExecutionSetup() {
},
},
},
}},
},
},
{
Name: "numbers_count",
Parameter: &core.Parameter{Var: &core.Variable{
"numbers_count": {
Var: &core.Variable{
Type: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_INTEGER,
},
},
}},
},
},
{
Name: "run_local_at_count",
Parameter: &core.Parameter{Var: &core.Variable{
"run_local_at_count": {
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 []*core.VariableMapEntry) (map[string]*core.Literal, error) {
func MakeLiteralForVariables(serialize map[string]interface{}, variables map[string]*core.Variable) (map[string]*core.Literal, error) {
types := make(map[string]*core.LiteralType)
for _, e := range variables {
t := e.GetVar().GetType()
for k, v := range variables {
t := v.GetType()
if t == nil {
return nil, fmt.Errorf("variable [%v] has nil type", e.GetName())
return nil, fmt.Errorf("variable [%v] has nil type", k)
}

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

return MakeLiteralForTypes(serialize, types)
Expand All @@ -28,15 +28,15 @@ func MakeLiteralForVariables(serialize map[string]interface{}, variables []*core
// 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 []*core.ParameterMapEntry) (map[string]*core.Literal, error) {
func MakeLiteralForParams(serialize map[string]interface{}, parameters map[string]*core.Parameter) (map[string]*core.Literal, error) {
types := make(map[string]*core.LiteralType)
for _, e := range parameters {
if variable := e.GetParameter().GetVar(); variable == nil {
return nil, fmt.Errorf("parameter [%v] has nil Variable", e.GetName())
for k, v := range parameters {
if variable := v.GetVar(); variable == nil {
return nil, fmt.Errorf("parameter [%v] has nil Variable", k)
} else if t := variable.GetType(); t == nil {
return nil, fmt.Errorf("parameter [%v] has nil variable type", e.GetName())
return nil, fmt.Errorf("parameter [%v] has nil variable type", k)
} else {
types[e.GetName()] = t
types[k] = t
}
}

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

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

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

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

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

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

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

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

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

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

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

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

if err != nil {
Expand Down
13 changes: 4 additions & 9 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 := []*core.VariableMapEntry{}
taskInputs := map[string]*core.Variable{}
t.Run("nil task", func(t *testing.T) {
retValue := TaskInputs(nil)
assert.Equal(t, taskInputs, retValue)
Expand Down Expand Up @@ -60,14 +60,9 @@ func createTask() *admin.Task {
},
}

variableMap := []*core.VariableMapEntry{
{
Name: "sorted_list1",
Var: &sortedListLiteralType,
}, {
Name: "sorted_list2",
Var: &sortedListLiteralType,
},
variableMap := map[string]*core.Variable{
"sorted_list1": &sortedListLiteralType,
"sorted_list2": &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[0].parameter.var.description"},
{Header: "Outputs", JSONPath: "$.closure.expectedOutputs.variables[0].var.description"},
{Header: "Inputs", JSONPath: "$.closure.expectedInputs.parameters." + printer.DefaultFormattedDescriptionsKey + ".var.description"},
{Header: "Outputs", JSONPath: "$.closure.expectedOutputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"},
}

// Column structure for get all launchplans
Expand Down
Loading

0 comments on commit 8db28dd

Please sign in to comment.