Add --ref support for experimental artifacts

[ebeahan]

Summary of Changes

When --ref argument is set and the experimental/schemas directory is loaded with --include, these changes ensure the experimental field definitions are pulling from the target git ref and not the local file system.

Background

--ref is restricted to loading the field definitions from the schemas directory. Custom field definition can be supplied but will always load from the local file system. This works well for users managing independent custom definitions outside of the ECS field definitions.

The experimental/schemas directory was introduced to hold ECS field definition changes considered experimental, but not beta or GA. Users can use --include experimental/schemas if they wish to generate artifacts using these experimental fields.

A problem is introduced if a user wishes to generate artifacts released for a specific version of ECS and include the experimental artifacts from that same version.

Example

A user, starting from scratch by cloning the ECS repo and installing dependencies, attempts to generate artifacts for a specific version:

$ git clone https://github.com/elastic/ecs
$ cd ecs
$ make ve
$ python scripts/generator.py --ref v1.7.0 --include experimental/schemas --out ../my-project/

If the v1.7.0 exists, --ref will properly checkout that git tree and load the field definitions in the schemas directory. However, --include loads custom field definitions from the user's local file system. The user would have inadvertently merged v1.7.0 fields with the latest experimental fields in the repo's default branch.

Proposed Solution

Add logic in schema.loader.load_schemas for when a --ref is provided and --include includes experiment/schemas, the contents of experiment/schemas is loaded from the target git ref. Other custom field definitions will continue to be loaded from the local file system.

[webmat]

I like that this is a straightforward fix to this problem, this could have gone in so many different directions.

My spidey sense is a bit leery because behind the scenes we're making --include work differently for one specific directory. This might be hard to convey in the Arg Parser description.

Perhaps we can try is to make a quick mention of this on the --ref arg?

We definitely need to explain this in the Usage section for --ref and --include.

After adding these two mentions above, and a suggested tweak to the exception we raise (below), I think we'll be good to move forward.

[ebeahan]

After adding these two mentions above, and a suggested tweak to the exception we raise (below), I think we'll be good to move forward.

Thanks @webmat! I made the suggested adjustments updated the usage documentation and argument descriptions.

After some back and forth, I landed on placing the new usage details underneath --ref since I think this is the best logical location.

[webmat]

Looking good!

My review is mostly about how we message this tricky situation. Let me know what you think of these suggestions.

The implementation looks good, and thanks for all the tests <3

[ebeahan]

Thanks @webmat - I've incorporated your suggestions.

After all the rephrasing/rewording, the exact lines you suggested removing shifted quite a bit. Let me know if the latest revisions weren't quite what you envisioned.

[webmat]

Thanks for the adjustments! LGTM