-
Notifications
You must be signed in to change notification settings - Fork 72
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
Adds support for templates index pattern #102
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
|
@jalvz Can you share a bit more details on how this will be used? |
@ruflin you have been too fast :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify the "good" test package, so we can be sure that the change works as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently blocking this PR as we should first complete the discussion in #64 (comment)
I like the simplicity of this change but I worry it has many side effects. On the Stack side we loose the knowledge if the templates / mappings / index patterns for a data stream are all aligned or if a dev might have just introduced an invalide index pattern.
Updated as per #64 (comment) |
@skh @jfsiii @jen-huang Could you chime in from the Kibana side if this is feasible? |
Filed Kibana issue: elastic/kibana#88307 |
@@ -111,6 +111,10 @@ spec: | |||
type: string | |||
examples: | |||
- diagnostics | |||
dataset_is_prefix: | |||
description: if true, the generated index pattern will contain the dataset as a prefix only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only about the field index_patterns
in an ES index template (this is what I guessed from the discussions), or also about Kibana index patterns (this is how this line reads without context)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is only about the ES index patterns. It should not affect the Kibana ones as these are based on the types and include all datasets anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this description should be changed accordingly.
Hey all, are there any more objections to this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but let's leave the final decision to @ruflin .
EDIT:
please rebase the branch against master and update the changelog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mtojek @ycombinator Side discussion: Should this go into https://github.com/elastic/package-spec/blob/master/versions/1/changelog.yml or where is the right place for this?
Yes, this is the right place. |
See motivation in #64 (comment)
It will require Kibana support, basically if this new property is
true
, the index pattern generated for the templates should be<type>-<dataset>.*-*
I will open a ticket in Kibana if this is accepted.