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

[fleet] Add to_yaml helper #136723

Closed
wants to merge 3 commits into from
Closed

Conversation

jeniawhite
Copy link
Contributor

@jeniawhite jeniawhite commented Jul 20, 2022

Summary

Adds to_yaml helper to handlebars for fleet server.

Checklist

For maintainers

@jeniawhite jeniawhite requested a review from a team as a code owner July 20, 2022 13:43
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 20, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jeniawhite jeniawhite added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Jul 20, 2022
@nchaulet
Copy link
Member

nchaulet commented Jul 20, 2022

Hi @jeniawhite we added recently a to_json helper #135992 I think it could probably be used in your template (I will be happy to help if needed here) instead of a new to_yaml helper

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 20, 2022

💔 Build Failed

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
apm 14 13 -1
observability 6 5 -1
securitySolution 74 73 -1
total -3

ESLint disabled line counts

id before after diff
apm 88 85 -3
observability 47 46 -1
synthetics 61 55 -6
ux 10 9 -1
total -11

Total ESLint disabled count

id before after diff
apm 102 98 -4
observability 53 51 -2
securitySolution 499 498 -1
synthetics 67 61 -6
ux 13 12 -1
total -14

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

case 'object':
// Null classified as object
if (isNil(value)) return;
const safeValue = replaceUndefinedObjectValues(value, true);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to replace undefined by null? I think there is still value to support undefined value if we want some keys to not be in the final yaml no?

Copy link
Contributor Author

@jeniawhite jeniawhite Jul 28, 2022

Choose a reason for hiding this comment

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

Passing objects with undefined key values causes safeDump to throw an exception.

> yaml.safeDump({key: undefined})
Uncaught:
YAMLException: unacceptable kind of an object to dump [object Undefined]
    at writeNode (/Users/evgb/Documents/GitHub/kibana/node_modules/js-yaml/lib/js-yaml/dumper.js:779:13)
    at writeBlockMapping (/Users/evgb/Documents/GitHub/kibana/node_modules/js-yaml/lib/js-yaml/dumper.js:657:10)
    at writeNode (/Users/evgb/Documents/GitHub/kibana/node_modules/js-yaml/lib/js-yaml/dumper.js:750:9)
    at dump (/Users/evgb/Documents/GitHub/kibana/node_modules/js-yaml/lib/js-yaml/dumper.js:840:7)
    at Object.safeDump (/Users/evgb/Documents/GitHub/kibana/node_modules/js-yaml/lib/js-yaml/dumper.js:846:10) {
  reason: 'unacceptable kind of an object to dump [object Undefined]',
  mark: undefined
}

@nchaulet nchaulet self-requested a review July 25, 2022 12:59
@jeniawhite
Copy link
Contributor Author

@nchaulet
Sorry for the late response, thank you very much for the feedback.
Due to planning changes, I will close this PR.

@jeniawhite jeniawhite closed this Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants