-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Nasir.thomas/AI-5146/ddev validate config fields #21744
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
Changes from 34 commits
7db1b0c
906e138
d4ae0d6
32ccc7f
62f4a94
f27a8a8
03f326b
f955f3d
2841b4a
ca44bf1
69f8773
0bec013
45e7aa4
245b6d8
43244a7
19347f4
9ce6d33
9fb226b
5e154ec
d3ed422
9f33042
e8c064d
2cfe2e9
b673e62
a99dcd6
0e6f7a5
db5dfd5
79872b1
290f6da
0df6e91
9f5ef95
2b531f9
0b52586
a31c6c0
b28ae19
79dc34d
29f391e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Update conf.yaml to display 'default' field defined in spec.yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added validation for option and value level fields in the ddev validate command. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,28 @@ | |
|
|
||
| import yaml | ||
|
|
||
| from datadog_checks.dev.tooling.configuration.constants import OPENAPI_SCHEMA_PROPERTIES | ||
|
|
||
| ALLOWED_OPTION_FIELDS = { | ||
| 'name', | ||
| 'description', | ||
| 'required', | ||
| 'hidden', | ||
| 'display_priority', | ||
| 'deprecation', | ||
| 'multiple', | ||
| 'multiple_instances_defined', | ||
| 'metadata_tags', | ||
| 'options', | ||
| 'value', | ||
| 'secret', | ||
| 'enabled', | ||
| 'example', | ||
| 'template', | ||
| 'overrides', | ||
| 'fleet_configurable', | ||
| } | ||
| ALLOWED_VALUE_FIELDS = OPENAPI_SCHEMA_PROPERTIES | {'example', 'display_default', 'compact_example'} | ||
| DESCRIPTION_LINE_LENGTH_LIMIT = 120 | ||
|
|
||
|
|
||
|
|
@@ -66,6 +88,18 @@ def option_enabled(option): | |
| return option['required'] | ||
|
|
||
|
|
||
| def validate_fields( | ||
| fields_dict: dict[str, any], option_name: str, allowed_fields: set[str], field_level: str, writer: OptionWriter | ||
| ): | ||
| invalid_fields = [field for field in fields_dict if field not in allowed_fields] | ||
|
|
||
| if invalid_fields: | ||
| invalid_fields_str = '\n'.join(f" - {field!r}" for field in invalid_fields) | ||
| writer.new_error( | ||
| f"Option name {option_name!r} contains the following invalid {field_level} fields:\n{invalid_fields_str}" | ||
| ) | ||
|
|
||
|
|
||
| def write_description(option, writer, indent, option_type): | ||
| description = option['description'] | ||
| deprecation = option['deprecation'] | ||
|
|
@@ -103,6 +137,9 @@ def write_description(option, writer, indent, option_type): | |
|
|
||
| def write_option(option, writer, indent='', start_list=False): | ||
| option_name = option['name'] | ||
|
|
||
| validate_fields(option, option_name, ALLOWED_OPTION_FIELDS, 'option-level', writer) | ||
|
|
||
| if 'value' in option: | ||
| value = option['value'] | ||
| required = option['required'] | ||
|
|
@@ -116,11 +153,13 @@ def write_option(option, writer, indent='', start_list=False): | |
| 'required' if required else 'optional', | ||
| ) | ||
|
|
||
| validate_fields(value, option_name, ALLOWED_VALUE_FIELDS, 'value-level', writer) | ||
|
|
||
| example = value.get('example') | ||
| example_type = type(example) | ||
| if not required: | ||
| if 'display_default' in value: | ||
| default = value['display_default'] | ||
| default = value.get('display_default', value.get('default')) | ||
| if default is not None: | ||
| default_type = type(default) | ||
| if default is not None and str(default).lower() != 'none': | ||
| if default_type is str: | ||
|
|
@@ -129,7 +168,7 @@ def write_option(option, writer, indent='', start_list=False): | |
| writer.write(' - default: ', 'true' if default else 'false') | ||
| else: | ||
| writer.write(' - default: ', repr(default)) | ||
| else: | ||
| elif 'display_default' not in value or 'default' in value: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: I am not sure I understand this statement in the Since the previous block only executes when Shouldn't this be just an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a weird issue I encountered when I left it as I believe this is because of the line If the key exists and the value is set to |
||
| if example_type is bool: | ||
| writer.write(' - default: ', 'true' if example else 'false') | ||
| elif example_type in (int, float): | ||
|
|
@@ -261,6 +300,24 @@ def render(self): | |
| if i != num_options: | ||
| writer.write('\n') | ||
|
|
||
| if writer.errors: | ||
| has_option_level_errors = any('option-level' in error for error in writer.errors) | ||
| has_value_level_errors = any('value-level' in error for error in writer.errors) | ||
|
|
||
| if has_option_level_errors or has_value_level_errors: | ||
| valid_fields = [] | ||
|
|
||
| if has_option_level_errors: | ||
| fields_list = '\n'.join(f" - {field!r}" for field in sorted(ALLOWED_OPTION_FIELDS)) | ||
| valid_fields.append(f"Option-level fields must be one of the following:\n{fields_list}") | ||
|
|
||
| if has_value_level_errors: | ||
| fields_list = '\n'.join(f" - {field!r}" for field in sorted(ALLOWED_VALUE_FIELDS)) | ||
| valid_fields.append(f"Value-level fields must be one of the following:\n{fields_list}") | ||
|
|
||
| if valid_fields: | ||
| writer.errors.append('\n'.join(valid_fields)) | ||
|
|
||
| files[file['example_name']] = (writer.contents, writer.errors) | ||
|
|
||
| return files | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Update conf.yaml to display 'default' field defined in spec.yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Update conf.yaml to display 'default' field defined in spec.yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,6 @@ files: | |
| enabled: true | ||
| value: | ||
| example: tls_only | ||
| default: false | ||
| anyOf: | ||
| - type: boolean | ||
| - type: string | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if the
ALLOWED_OPTION_FIELDSwould be better suited indatadog_checks.dev.tooling.configuration.constantssince the file seemed specific to Open API constants. Please let me know if I should move it. Thanks