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

Add option for custom template and mapping settings #856

Merged
merged 4 commits into from
May 21, 2020

Conversation

marshallmain
Copy link
Contributor

@marshallmain marshallmain commented May 20, 2020

Little quality of life improvement that lets users override the default template and mapping settings for the generated ES templates. This PR adds 2 new options, template-settings and mapping-settings that take in filenames and use those json files to populate the template and mapping settings of the generated ES template.

Example usage, run in the ECS directory:
python scripts/generator.py --mapping-settings custom_mapping.json --template-settings custom_template.json
This replaces the mapping and template settings with whatever is in custom_mapping.json and custom_template.json, respectively.

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

That's a great addition, thanks Marshall :-)

Since this is only used by the ES template generator, would you mind passing the args to it as is, and load the JSONs from there?

Also, would you mind providing a usage example as a comment on the PR?

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Thanks for the adjustments. One more nitpick and then you can merge :-)

@@ -3,6 +3,7 @@
import os
import schema_reader
import yaml
import json
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Please remove this new import, as it's no longer needed :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good point

Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

Reviewed and tested out new functionality - LGTM

@ebeahan ebeahan merged commit 27fed93 into elastic:master May 21, 2020
@marshallmain marshallmain deleted the custom-templates branch May 21, 2020 23:36
@ebeahan ebeahan mentioned this pull request Jul 9, 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.

3 participants