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.

Sign up for GitHub

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

Jump to bottom

mlos_bench config json schema validation: optimizers and tunable_values #340

Merged
merged 72 commits into from
May 15, 2023
Merged

mlos_bench config json schema validation: optimizers and tunable_values #340

merged 72 commits into from
May 15, 2023

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented May 8, 2023 •
edited
Loading

This PR introduces initial json schemas for mlos_bench optimizer configs and tunable_values configs, their validation at load time, and tests for both.

Future PRs will handle other config types.
See Also: #331

bpkroth
bpkroth commented May 8, 2023
View reviewed changes
.vscode/settings.json Outdated Show resolved Hide resolved
@bpkroth bpkroth mentioned this pull request May 8, 2023
Merged
This was referenced May 9, 2023
Closed
Closed
@bpkroth bpkroth changed the title json schema validation json schema validation: optimizers May 9, 2023
motus
motus requested changes May 13, 2023
View reviewed changes
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.

It looks like we remove "$schema" from all JSONs - including the ARM template. As a result, our vm_provision() service method fails when invoking Azure API. Here's the log:

2023-05-12 17:00:47,682 azure_services.py:442 vm_provision DEBUG Request: PUT https://management.azure.com/subscriptions/84334f8c-9e72-424d-8fc2-fd6da32e9ad6/resourceGroups/os-autotune/providers/Microsoft.Resources/deployments/os-autotune-001?api-version=2022-05-01
...
2023-05-12 17:00:48,087 connectionpool.py:456 _make_request DEBUG https://management.azure.com:443 "PUT /subscriptions/84334f8c-9e72-424d-8fc2-fd6da32e9ad6/resourceGroups/os-autotune/providers/Microsoft.Resources/deployments/os-autotune-001?api-version=2022-05-01 HTTP/1.1" 400 218
2023-05-12 17:00:48,089 azure_services.py:449 vm_provision DEBUG Response: <Response [400]>
{
  "error": {
    "code": "InvalidRequestContent",
    "message": "The request content was invalid and could not be deserialized: 'Required property '$schema' not found in JSON. Path 'properties.template', line 1, position 6360.'."
  }
}

Our main branch currently works end-to-end and I really want to keep it in a working state so I can run

mlos_bench --config .\mlos_bench\mlos_bench\config\cli\azure-redis-skopt.jsonc

from master at any time. Is it possible with this PR? (also, make it pass flake8/pylint/mypy checks)

mlos_bench/mlos_bench/services/config_persistence.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/services/config_persistence.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/services/config_persistence.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/services/config_persistence.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/services/remote/azure/azure_services.py Show resolved Hide resolved
@motus
Copy link
Member

motus commented May 13, 2023

BTW many flake8 and pylint fixes are already in #354

@bpkroth bpkroth mentioned this pull request May 15, 2023
Closed
motus
motus approved these changes May 15, 2023
View reviewed changes
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.

Our redis+azure example works again! Left a few flake8+pylint comments; otherwise APPROVE

mlos_bench/mlos_bench/tests/config/environments/test_load_environment_config_examples.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/schemas/__init__.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/schemas/optimizers/test_optimizer_schemas.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/storage/test_load_storage_config_examples.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/services/test_load_service_config_examples.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/schemas/__init__.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/tests/config/schemas/__init__.py Show resolved Hide resolved
@bpkroth bpkroth merged commit db1a0e3 into microsoft:main May 15, 2023
@bpkroth bpkroth deleted the json-schema-validation branch May 15, 2023 21:30
bpkroth added a commit that referenced this pull request May 16, 2023
@bpkroth
mlos_bench config json schema validation - tunable params (#346)
fbec7d7 
Extends #340 to tunable params.
See Also: #331
@bpkroth bpkroth mentioned this pull request May 16, 2023
Merged
3 tasks
bpkroth added a commit that referenced this pull request May 17, 2023
@bpkroth
mlos_bench config json schema validation - cli (#352)
29c77c9 
Extends #340, #346, #349 to cli configs.
See Also: #331 

Also updates `--log_level` to support named logging levels (e.g. `INFO`)

- [x] add tests
  - [x] also backfilling tests of loading cli config examples (similar to what #313 did)
- [x] global config handling
@bpkroth bpkroth mentioned this pull request May 19, 2023
Merged
3 tasks
bpkroth added a commit that referenced this pull request May 19, 2023
@bpkroth
mlos_bench config json schema validation - environments (#359)
0ff623b 
Extends #340, #346, #349, #352 to Environment configs.
See Also: #331 

- [x] main schema as a set of subschemas
- [x] connect schema validation in the mlos_bench code
- [x] test-cases
bpkroth added a commit that referenced this pull request May 22, 2023
@bpkroth
mlos_bench config json schema validation - services (#368)
f7348f1 
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
Reviewers

@motus motus motus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bpkroth @motus