Add build-version option for building specific versions of ECS #851
Conversation
scripts/generator.py
Outdated
| @@ -79,12 +87,12 @@ def argument_parser(): | |||
| parser.add_argument('--subset', nargs='+', | |||
| help='render a subset of the schema') | |||
| parser.add_argument('--out', action='store', help='directory to store the generated files') | |||
| parser.add_argument('--build-version', action='store', help='version of official ECS schemas to use') | |||
There was a problem hiding this comment.
The wording here is confusing. I think we should simply use a Git centric --ref instead.
While the goal is indeed to be able to build on top of specific versions, the implementation is actually just loading git refs. Aligning the wording will make sure users understand when it doesn't work (e.g. "oh there's no git ref of that name") and will make it clear that it can be used in other awesome ways, like building on top of a feature branch.
scripts/tests/test_ecs_spec.py
Outdated
| versions = ['master', 'v1.5.0'] | ||
| self.ecs_nested_schemas = [] | ||
| self.ecs_flat_schemas = [] | ||
| for version in versions: | ||
| tree = ecs_helpers.get_git_tree(version) | ||
| schemas = schema_reader.load_schemas_from_git(tree) | ||
| intermediate_schemas = schema_reader.create_schema_dicts(schemas) | ||
| schema_reader.assemble_reusables(intermediate_schemas) | ||
| (nested, flat) = schema_reader.generate_nested_flat(intermediate_schemas) | ||
| self.ecs_nested_schemas.append(nested) | ||
| self.ecs_flat_schemas.append(flat) |
There was a problem hiding this comment.
The intent of the test_ecs_spec.py test file is to make high level sanity checks and assertions about the schema. As such, it's important that we're able to keep adding things in here about the current version we're building (e.g. ensuring that geo is correctly nested in a new place). As such, we can't have this test happen based on reading other git refs entirely. Once again, think about this in terms of running tests from within a feature branch.
Please move the unit tests about reading git refs to the test_ecs_helpers.py file instead.
The cleanup of moving the schema loading to setUp is a good one, though. We can keep that 😄
Makefile
Outdated
| @@ -61,7 +62,7 @@ generate: legacy_use_cases codegen generator | |||
| # Run the new generator | |||
| .PHONY: generator | |||
| generator: | |||
| $(PYTHON) scripts/generator.py --include "${INCLUDE}" | |||
| $(PYTHON) scripts/generator.py --include "${INCLUDE}" --build-version ${BUILD_VERSION} | |||
There was a problem hiding this comment.
make generator doesn't need to specify a git ref, it should by default run on the files that are already checked out.
scripts/generator.py
Outdated
| version = 'master' | ||
| if args.build_version: | ||
| version = args.build_version | ||
| tree = ecs_helpers.get_git_tree(version) | ||
| ecs_version = read_version(tree) |
There was a problem hiding this comment.
Please align the wording to "git refs" here as well.
There was a problem hiding this comment.
This is great, thanks Marshall!
I tried it and it works well for 1.4 too :-D
Doesn't work for 1.3 and before because the asciidoc expects attributes that didn't exist back then (categorization stuff), but that's out of scope for this PR. If it comes up we'll address in a follow-up.
Trivial conflict in changelog. Interesting conflict with elastic#851. The new scripts/schema/loader.py file has most of the new code around loading ECS files from git refs. In this merge commit I'm also slightly adjusting comments and wording of output messages around this.
Closes #837
This provides an easy way to use the upgraded tooling in master to build any version of the ECS schemas. The new
build-versioncommand line option accepts both tags, such asv1.5.0, and commit hashes, such as1454f8b. This makes it easy to use new tooling features related to custom schemas with a stable released version of the official schema.