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

mlos_bench config json schema validation - environments #359

Merged
merged 192 commits into from
May 19, 2023

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented May 16, 2023

Extends #340, #346, #349, #352 to Environment configs.
See Also: #331

  • main schema as a set of subschemas
  • connect schema validation in the mlos_bench code
  • test-cases

@bpkroth bpkroth marked this pull request as ready for review May 19, 2023 16:56
@bpkroth bpkroth requested a review from a team as a code owner May 19, 2023 16:56
@bpkroth bpkroth changed the title WIP: mlos_bench config json schema validation - environments mlos_bench config json schema validation - environments May 19, 2023
Copy link
Member

@motus motus left a comment

Choose a reason for hiding this comment

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

Left a few comments, but overall looks great!

@bpkroth bpkroth enabled auto-merge (squash) May 19, 2023 21:13
@bpkroth bpkroth merged commit 0ff623b into microsoft:main May 19, 2023
@bpkroth bpkroth deleted the json-schema-validation-env branch May 19, 2023 23:24
bpkroth added a commit that referenced this pull request May 22, 2023
Extends #340, #346, #349, #352, #359 to Service configs.
See Also: #331 

- [x] main schema as a set of subschemas
- [x] connect schema validation in the mlos_bench code
- [x] test-cases

To do this we removed support for configs that were flat lists of service configs: `[ {"class": "service.class"} ]` and turned those into objects that have a single key `"services"` with the list instead. `{ "services": [ ... ] }`
This makes sure that all configs are dicts/objects that we can optionally put `"$schema"` attributes on but also simplifies some of the config loading logic.
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.

2 participants