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

feat: [v2] Config conversion and validation code from one data file #677

Merged
merged 19 commits into from
May 15, 2023

Conversation

kentquirk
Copy link
Contributor

@kentquirk kentquirk commented May 9, 2023

This is the beginnings of a multi-pronged approach to data conversion.

We want a tool that can read an existing V1 config file and generate a new one, omitting no-longer-needed values and reformatting others, and generating a properly commented YAML file that is up to date with Refinery's capabilities.

We also want a validation library that can check a config completely. The best way to do that is with a JSON Schema validator, which in turn needs a JSON Schema data file that expresses the constraints on the fields.

Since both of these things need a lot of information about all the fields in the config, this PR creates one big data file that will contain enough information to solve both problems, and I'm planning to generate both the template for the converter and the JSON schema from the same data file.

In scope for this PR:

  • create config converter that uses a template to convert
  • create data file containing details of config items
  • create tool for generating the template from the data file
  • create rough structure of new config
  • manually check every field
  • settle on the final design of config structure
  • adjust config structure to that design
  • finish config converter tool
  • document config converter tool

NOT in scope:

  • adapt Refinery's config structure to this new design
  • do anything with validation schema (this includes additional annotations to support schema -- these will come later)

@kentquirk kentquirk self-assigned this May 10, 2023
@kentquirk kentquirk added the type: enhancement New feature or request label May 10, 2023
@kentquirk kentquirk added this to the v2.0 milestone May 10, 2023
@kentquirk kentquirk marked this pull request as ready for review May 11, 2023 22:43
@kentquirk kentquirk requested a review from a team as a code owner May 11, 2023 22:43
@kentquirk
Copy link
Contributor Author

The config format and this tool are now in good enough shape to move on to other things; we will revisit before final release.

@kentquirk
Copy link
Contributor Author

There is now a single executable with subcommands(rather than running things as tests) with a Makefile to drive the non-converter commands. I've also done a go:embed so that the executable stands alone without need for external template or data files. And I've put summaries back and edited the descriptions so they're not redundant to the summaries.

@kentquirk
Copy link
Contributor Author

I just added docs generation for config; it writes out config.md.

tools/convert/Makefile Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
tools/convert/README.md Outdated Show resolved Hide resolved
tools/convert/configData.yaml Show resolved Hide resolved
tools/convert/configV2.tmpl Outdated Show resolved Hide resolved
tools/convert/templates/configV2.tmpl Outdated Show resolved Hide resolved
@kentquirk kentquirk requested a review from TylerHelmuth May 15, 2023 20:00
@kentquirk kentquirk merged commit 36e0126 into main May 15, 2023
@kentquirk kentquirk deleted the kent.v2.convert_and_validate branch May 15, 2023 21:11
kentquirk added a commit that referenced this pull request May 17, 2023
## Which problem is this PR solving?

- After Honeycomb introduced environments, the rules file uses
environments to specify samplers but still refers to things in code,
tests, and documentation as datasets. This fixes the code part --
forthcoming v2 changes will fix the tests and documentation.

## Short description of the changes

- Rename GetSamplerConfigForDataset to GetSamplerConfigForDestName and
change its internals as well.
- ReloadInto was exported because it's needed for the converter

This PR depends on #677 and should be merged after that one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants