Skip to content

Conversation

@ruflin
Copy link
Collaborator

@ruflin ruflin commented May 11, 2020

No description provided.


dataset: {{dataset}}

{{custom}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nchaulet I tried to have a fully custom part in this config here. When I try to fill it in with foo: bar in the UI I get the following error:

can not read an implicit mapping pair; a colon is missed at line 7, column 13: "##custom##" ^

If would assume it replaced {{custom}} by foo: bar so it should be valid?

Copy link
Member

@nchaulet nchaulet May 11, 2020

Choose a reason for hiding this comment

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

This a good scenario that we did not cover, I did the compilation of yaml to be able to support values like custom: {{custom}}

Thinking of the solution we have:

  1. We can probably add a new concept in the registry variables, to flag a variable that should be spread to the agent yaml.
  2. we can try to detect in the template that custom is a root level yaml value and do another special treatment, I have a POC here [Ingest] Support root level yaml variables in agent stream template kibana#66120

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

++ on option 2 as I would prefer to have as few "types" as possible on the package level.

@elasticmachine
Copy link

elasticmachine commented May 11, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Branch indexing]

  • Start Time: 2020-05-18T12:18:14.748+0000

  • Duration: 4 min 13 sec (252546)

@ruflin ruflin marked this pull request as ready for review May 18, 2020 15:07
@ruflin ruflin requested a review from ph May 18, 2020 16:00
Copy link

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM.

@ruflin ruflin merged commit 96c3330 into elastic:master May 18, 2020
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.

4 participants