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

Fix typo detection for config field aliases #12468

Merged
merged 9 commits into from
Jul 11, 2022
Merged

Fix typo detection for config field aliases #12468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ee6e7fc
Test the typos returned by the log_typos_in_options method
FlorentClarret Jul 7, 2022
86aa1cf
Take aliases into account when looking for typos in options
FlorentClarret Jul 7, 2022
dfaff82
Fix for Python 2
FlorentClarret Jul 7, 2022
9b9fa38
Run black
FlorentClarret Jul 7, 2022
1164484
Ugly fix for Python 2 tests
FlorentClarret Jul 7, 2022
d1602ee
Run black
FlorentClarret Jul 8, 2022
5e9e369
Fix for mypy
FlorentClarret Jul 8, 2022
1ae00a2
Add a unit test with a typo in an alias
FlorentClarret Jul 8, 2022
719c418
Move imports
FlorentClarret Jul 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions datadog_checks_base/datadog_checks/base/checks/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
monkey_patch_pyyaml()

if not PY2:
from pydantic import ValidationError
from pydantic import BaseModel, ValidationError

if TYPE_CHECKING:
import ssl
Expand Down Expand Up @@ -443,7 +443,14 @@ def log_typos_in_options(self, user_config, models_config, level):
models_config = models_config or {}
typos = set() # type: Set[str]

known_options = [k for k, _ in models_config] # type: List[str]
known_options = set([k for k, _ in models_config]) # type: Set[str]

if not PY2:

if isinstance(models_config, BaseModel):
# Also add aliases, if any
known_options.update(set(models_config.dict(by_alias=True)))

unknown_options = [option for option in user_configs.keys() if option not in known_options] # type: List[str]

for unknown_option in unknown_options:
Expand Down
210 changes: 165 additions & 45 deletions datadog_checks_base/tests/base/checks/test_agent_check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,106 +1043,226 @@ def test_load_configuration_models(dd_run_check, mocker):
assert check._config_model_shared is shared_config


if PY3:

from pydantic import BaseModel, Field

class BaseModelTest(BaseModel):
field = ""
schema_ = Field("", alias='schema')

else:

class BaseModelTest:
def __init__(self, **kwargs):
pass


@requires_py3
@pytest.mark.parametrize(
'check_instance_config, default_instance_config, log_lines',
"check_instance_config, default_instance_config, log_lines, unknown_options",
[
pytest.param(
{'endpoint': 'url', 'tags': ['foo:bar'], 'proxy': {'http': 'http://1.2.3.4:9000'}},
{
"endpoint": "url",
"tags": ["foo:bar"],
"proxy": {"http": "http://1.2.3.4:9000"},
},
[],
None,
id='empty default',
[],
id="empty default",
),
pytest.param(
{'endpoint': 'url', 'tags': ['foo:bar'], 'proxy': {'http': 'http://1.2.3.4:9000'}},
[('endpoint', 'url')],
{
"endpoint": "url",
"tags": ["foo:bar"],
"proxy": {"http": "http://1.2.3.4:9000"},
},
[
("endpoint", "url"),
],
None,
id='no typo',
[],
id="no typo",
),
pytest.param(
{'endpoints': 'url', 'tags': ['foo:bar'], 'proxy': {'http': 'http://1.2.3.4:9000'}},
[('endpoint', 'url')],
{
"endpoints": "url",
"tags": ["foo:bar"],
"proxy": {"http": "http://1.2.3.4:9000"},
},
[
("endpoint", "url"),
],
[
(
'Detected potential typo in configuration option in test/instance section: `endpoints`. '
'Did you mean endpoint?'
"Detected potential typo in configuration option in test/instance section: `endpoints`. "
"Did you mean endpoint?"
)
],
id='typo',
["endpoints"],
id="typo",
),
pytest.param(
{'endpoints': 'url', 'tags': ['foo:bar'], 'proxy': {'http': 'http://1.2.3.4:9000'}},
[('endpoint', 'url'), ('endpoints', 'url')],
{
"endpoints": "url",
"tags": ["foo:bar"],
"proxy": {"http": "http://1.2.3.4:9000"},
},
[
("endpoint", "url"),
("endpoints", "url"),
],
None,
id='no typo similar option',
[],
id="no typo similar option",
),
pytest.param(
{'endpont': 'url', 'tags': ['foo:bar'], 'proxy': {'http': 'http://1.2.3.4:9000'}},
[('endpoint', 'url'), ('endpoints', 'url')],
{
"endpont": "url",
"tags": ["foo:bar"],
"proxy": {"http": "http://1.2.3.4:9000"},
},
[
("endpoint", "url"),
("endpoints", "url"),
],
[
(
'Detected potential typo in configuration option in test/instance section: `endpont`. '
'Did you mean endpoint, or endpoints?'
"Detected potential typo in configuration option in test/instance section: `endpont`. "
"Did you mean endpoint, or endpoints?"
)
],
id='typo two candidates',
["endpont"],
id="typo two candidates",
),
pytest.param(
{
"tag": "test",
},
[
("tags", "test"),
],
None,
[],
id="short option cant catch",
),
pytest.param({'tag': 'test'}, [('tags', 'test')], None, id='short option cant catch'),
pytest.param(
{'testing_long_para': 'test'},
[('testing_long_param', 'test'), ('test_short_param', 'test')],
{
"testing_long_para": "test",
},
[
("testing_long_param", "test"),
("test_short_param", "test"),
],
[
(
'Detected potential typo in configuration option in test/instance section: `testing_long_para`. '
'Did you mean testing_long_param?'
"Detected potential typo in configuration option in test/instance section: `testing_long_para`. "
"Did you mean testing_long_param?"
)
],
id='somewhat similar option',
["testing_long_para"],
id="somewhat similar option",
),
pytest.param(
{'send_distribution_sums_as_monotonic': False, 'exclude_labels': True},
[('send_distribution_counts_as_monotonic', True), ('include_labels', True)],
{
"send_distribution_sums_as_monotonic": False,
"exclude_labels": True,
},
[
("send_distribution_counts_as_monotonic", True),
("include_labels", True),
],
None,
id='different options no typos',
[],
id="different options no typos",
),
pytest.param(
{'send_distribution_count_as_monotonic': True, 'exclude_label': True},
{
"send_distribution_count_as_monotonic": True,
"exclude_label": True,
},
[
('send_distribution_sums_as_monotonic', False),
('send_distribution_counts_as_monotonic', True),
('exclude_labels', False),
('include_labels', True),
("send_distribution_sums_as_monotonic", False),
("send_distribution_counts_as_monotonic", True),
("exclude_labels", False),
("include_labels", True),
],
[
(
'Detected potential typo in configuration option in test/instance section: '
'`send_distribution_count_as_monotonic`. Did you mean send_distribution_counts_as_monotonic?'
"Detected potential typo in configuration option in test/instance section: "
"`send_distribution_count_as_monotonic`. Did you mean send_distribution_counts_as_monotonic?"
),
(
'Detected potential typo in configuration option in test/instance section: `exclude_label`. '
'Did you mean exclude_labels?'
"Detected potential typo in configuration option in test/instance section: `exclude_label`. "
"Did you mean exclude_labels?"
),
],
id='different options typo',
[
"send_distribution_count_as_monotonic",
"exclude_label",
],
id="different options typo",
),
pytest.param(
{
"field": "value",
"schema": "my_schema",
},
BaseModelTest(field="my_field", schema_="the_schema"),
None,
[],
id="using an alias",
),
pytest.param(
{
"field": "value",
"schem": "my_schema",
},
BaseModelTest(field="my_field", schema_="the_schema"),
[
(
"Detected potential typo in configuration option in test/instance section: "
"`schem`. Did you mean schema?"
),
],
["schem"],
id="typo in an alias",
),
pytest.param(
{
"field": "value",
"schema_": "my_schema",
},
BaseModelTest(field="my_field", schema_="the_schema"),
None,
[],
id="not using an alias",
),
],
)
def test_detect_typos_configuration_models(
dd_run_check, mocker, caplog, check_instance_config, default_instance_config, log_lines
dd_run_check,
caplog,
check_instance_config,
default_instance_config,
log_lines,
unknown_options,
):
caplog.clear()
caplog.set_level(logging.WARNING)
empty_config = {}
default_instance = mocker.MagicMock()
default_instance.__iter__ = mocker.MagicMock(return_value=iter(default_instance_config))

check = AgentCheck('test', empty_config, [check_instance_config])
check.check_id = 'test:123'
check = AgentCheck("test", empty_config, [check_instance_config])
check.check_id = "test:123"

check.log_typos_in_options(check_instance_config, default_instance, 'instance')
typos = check.log_typos_in_options(check_instance_config, default_instance_config, "instance")

if log_lines is not None:
for log_line in log_lines:
assert log_line in caplog.text
else:
assert 'Detected potential typo in configuration option' not in caplog.text
assert "Detected potential typo in configuration option" not in caplog.text

assert typos == set(unknown_options)