Skip to content

Conversation

zyy98
Copy link
Contributor

@zyy98 zyy98 commented Aug 16, 2022

To log results in Runner interface, current method does not supply Field struct, add a new method that logs Field as well.

@zyy98 zyy98 changed the title add result with field fix: add context result with field Aug 16, 2022
}
}

func ConfigObjectResultWithField(msg string, obj *KubeObject, severity Severity, field *Field) *Result {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am hoping this function can be smarter: user provides the field path and proposedValue, it can find out the currentValue from the path, determining the right type, and validate the proposedValue field

Also. ln304-317 can be simplified by calling ConfigObjectResult

Copy link
Contributor Author

@zyy98 zyy98 Aug 16, 2022

Choose a reason for hiding this comment

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

I think it is difficult to have a path and determine its currentValue, the path might contain a slice. How should I represent the slice using path? Should I add [] in the path?

Since I am hard coding the path inside my new transformer, I know in advance if this field is a slice or not. How should I convey this message to ConfigObjectResultWithField? For the cases with AdditionalLabelFields, it uses [] to indicate a slice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest reading the KRM function specification as I mentioned in last 1:1.

Path is the JSON path of the field e.g. spec.template.spec.containers[3].resources.limits.cpu

Copy link
Contributor Author

@zyy98 zyy98 Aug 18, 2022

Choose a reason for hiding this comment

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

On second thought, this is suppose to use when logging results. All the results should be updated before the logging. There is no point getting currentValue using path @yuwenma . In implementation, the old value and new value are stored in some map, then later used in the logging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants