From 8a6d442d2ba461fc21b521d56d679b32f7ebf65e Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 3 May 2024 20:06:10 +0200 Subject: [PATCH 1/8] hint about extra fields in conda-forge.yml --- conda_smithy/lint_recipe.py | 49 +++++++++++++++++++++++++++++++++---- conda_smithy/schema.py | 5 +++- conda_smithy/utils.py | 4 +-- tests/test_lint_recipe.py | 45 ++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 29a599f6d..2c81c0c7a 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -2,6 +2,10 @@ from collections.abc import Sequence, Mapping +from pydantic import BaseModel + +from conda_smithy.schema import ConfigModel + str_type = str import copy @@ -148,6 +152,28 @@ def find_local_config_file(recipe_dir, filename): return found_filesname[0] if found_filesname else None +def _forge_yaml_hint_extra_fields(forge_yaml: dict) -> list[str]: + """ + Identify unexpected keys in the conda-forge.yml file. + This only works if extra="allow" is set in the Pydantic sub-model where the unexpected key is found. + """ + + config = ConfigModel.model_validate(forge_yaml) + hints = [] + + def _find_extra_fields(model: BaseModel, prefix=""): + for extra_field in (model.__pydantic_extra__ or {}).keys(): + hints.append(f"Unexpected key {prefix + extra_field}") + + for field, value in model: + if isinstance(value, BaseModel): + _find_extra_fields(value, f"{prefix + field}.") + + _find_extra_fields(config) + + return hints + + def lintify_forge_yaml(recipe_dir=None) -> (list, list): if recipe_dir: forge_yaml_filename = ( @@ -168,7 +194,14 @@ def lintify_forge_yaml(recipe_dir=None) -> (list, list): forge_yaml = {} # This is where we validate against the jsonschema and execute our custom validators. - return validate_json_schema(forge_yaml) + json_lints, json_hints = validate_json_schema(forge_yaml) + + lints = [_format_validation_msg(err) for err in json_lints] + hints = [_format_validation_msg(hint) for hint in json_hints] + + hints.extend(_forge_yaml_hint_extra_fields(forge_yaml)) + + return lints, hints def lintify_meta_yaml( @@ -1395,10 +1428,12 @@ def main(recipe_dir, conda_forge=False, return_hints=False): return results -if __name__ == "__main__": - # This block is supposed to help debug how the rendered version - # of the linter bot would look like in Github. Taken from - # https://github.com/conda-forge/conda-forge-webservices/blob/747f75659/conda_forge_webservices/linting.py#L138C1-L146C72 +def main_debug(): + """ + This function is supposed to help debug how the rendered version + of the linter bot would look like in GitHub. Taken from + https://github.com/conda-forge/conda-forge-webservices/blob/747f75659/conda_forge_webservices/linting.py#L138C1-L146C72 + """ rel_path = sys.argv[1] lints, hints = main(rel_path, False, True) messages = [] @@ -1417,3 +1452,7 @@ def main(recipe_dir, conda_forge=False, return_hints=False): ) print(*messages, sep="\n") + + +if __name__ == "__main__": + main_debug() diff --git a/conda_smithy/schema.py b/conda_smithy/schema.py index 5f9833e47..568d5feb8 100644 --- a/conda_smithy/schema.py +++ b/conda_smithy/schema.py @@ -481,6 +481,7 @@ class DefaultTestPlatforms(StrEnum): platform.value: (Optional[Platforms], Field(default=platform.value)) for platform in Platforms }, + __config__=ConfigDict(extra="allow"), ) OSVersion = create_model( @@ -490,6 +491,7 @@ class DefaultTestPlatforms(StrEnum): for platform in Platforms if platform.value.startswith("linux") }, + __config__=ConfigDict(extra="allow"), ) ProviderType = Union[List[CIservices], CIservices, bool, Nullable] @@ -506,6 +508,7 @@ class DefaultTestPlatforms(StrEnum): for plat in ("linux_64", "osx_64", "win_64") ] ), + __config__=ConfigDict(extra="allow"), ) @@ -523,7 +526,7 @@ class ConfigModel(BaseModel): # Values which are not expected to be present in the model dump, are # flagged with exclude=True. This is to avoid confusion when comparing # the model dump with the default conda-forge.yml file used for smithy - # or to avoid deprecated values been rendered. + # or to avoid deprecated values being rendered. conda_build: Optional[CondaBuildConfig] = Field( default_factory=CondaBuildConfig, diff --git a/conda_smithy/utils.py b/conda_smithy/utils.py index 9faf3bdc1..d68955fd4 100644 --- a/conda_smithy/utils.py +++ b/conda_smithy/utils.py @@ -18,7 +18,7 @@ def get_feedstock_name_from_meta(meta): - """Resolve the feedtstock name from the parsed meta.yaml.""" + """Resolve the feedstock name from the parsed meta.yaml.""" if "feedstock-name" in meta.meta["extra"]: return meta.meta["extra"]["feedstock-name"] elif "parent_recipe" in meta.meta["extra"]: @@ -28,7 +28,7 @@ def get_feedstock_name_from_meta(meta): def get_feedstock_about_from_meta(meta) -> dict: - """Fetch the feedtstock about from the parsed meta.yaml.""" + """Fetch the feedstock about from the parsed meta.yaml.""" # it turns out that conda_build would not preserve the feedstock about: # - if a subpackage does not have about, it uses the feedstock's # - if a subpackage has about, it's used as is diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index b398320bb..cbaded859 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -1997,5 +1997,50 @@ def test_lint_wheels(tmp_path, yaml_block, annotation): assert any(expected_message in hint for hint in hints) +class TestLintifyForgeYamlHintExtraFields(unittest.TestCase): + def test_extra_build_platforms_platform(self): + forge_yml = { + "build_platform": { + "osx_64": "linux_64", + "UNKNOWN_PLATFORM": "linux_64", + } + } + + hints = linter._forge_yaml_hint_extra_fields(forge_yml) + + self.assertEquals(len(hints), 1) + + self.assertIn( + "Unexpected key build_platform.UNKNOWN_PLATFORM", hints[0] + ) + + def test_extra_os_version_platform(self): + forge_yml = { + "os_version": { + "UNKNOWN_PLATFORM_2": "10.9", + } + } + + hints = linter._forge_yaml_hint_extra_fields(forge_yml) + + self.assertEquals(len(hints), 1) + + self.assertIn("Unexpected key os_version.UNKNOWN_PLATFORM_2", hints[0]) + + def test_extra_provider_platform(self): + forge_yml = { + "provider": { + "osx_64": "travis", + "UNKNOWN_PLATFORM_3": "azure", + } + } + + hints = linter._forge_yaml_hint_extra_fields(forge_yml) + + self.assertEquals(len(hints), 1) + + self.assertIn("Unexpected key provider.UNKNOWN_PLATFORM_3", hints[0]) + + if __name__ == "__main__": unittest.main() From 0d5c7306d93bcedaacaac316bd75cef70f8e0ab3 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 3 May 2024 20:09:33 +0200 Subject: [PATCH 2/8] add news file --- news/1920-hint-extra-fields.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/1920-hint-extra-fields.rst diff --git a/news/1920-hint-extra-fields.rst b/news/1920-hint-extra-fields.rst new file mode 100644 index 000000000..f13f3101d --- /dev/null +++ b/news/1920-hint-extra-fields.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* ``recipe-lint`` now hints about additional ``conda-forge.yml`` fields that are not part of the schema + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* From d6307ea4a11c9130baab7137d95e4d9604c9efb2 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 3 May 2024 20:11:41 +0200 Subject: [PATCH 3/8] use outdated List type --- conda_smithy/lint_recipe.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 2c81c0c7a..c5fd3d7be 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from collections.abc import Sequence, Mapping +from typing import List from pydantic import BaseModel @@ -152,7 +153,7 @@ def find_local_config_file(recipe_dir, filename): return found_filesname[0] if found_filesname else None -def _forge_yaml_hint_extra_fields(forge_yaml: dict) -> list[str]: +def _forge_yaml_hint_extra_fields(forge_yaml: dict) -> List[str]: """ Identify unexpected keys in the conda-forge.yml file. This only works if extra="allow" is set in the Pydantic sub-model where the unexpected key is found. From 294ce7e48c78595fbafb57e3c94cf47902c4d3b7 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Sun, 5 May 2024 12:13:44 +0200 Subject: [PATCH 4/8] lintify_forge_yaml returns lists of str --- conda_smithy/lint_recipe.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index c5fd3d7be..be4b89a29 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -175,7 +175,7 @@ def _find_extra_fields(model: BaseModel, prefix=""): return hints -def lintify_forge_yaml(recipe_dir=None) -> (list, list): +def lintify_forge_yaml(recipe_dir=None) -> (List[str], List[str]): if recipe_dir: forge_yaml_filename = ( glob(os.path.join(recipe_dir, "..", "conda-forge.yml")) @@ -1420,8 +1420,8 @@ def main(recipe_dir, conda_forge=False, return_hints=False): recipe_dir=recipe_dir ) - results.extend([_format_validation_msg(err) for err in validation_errors]) - hints.extend([_format_validation_msg(hint) for hint in validation_hints]) + results.extend(validation_errors) + hints.extend(validation_hints) if return_hints: return results, hints From 106812528ff6aeff1bf42115c5b4ef5a5e8167d3 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 13 May 2024 16:27:54 +0200 Subject: [PATCH 5/8] use pytest, add tests for AzureRunnerSettings and CondaBuildConfig --- conda_smithy/lint_recipe.py | 1 - tests/test_lint_recipe.py | 43 +++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index be4b89a29..0ba65a432 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -1439,7 +1439,6 @@ def main_debug(): lints, hints = main(rel_path, False, True) messages = [] if lints: - all_pass = False messages.append( "\nFor **{}**:\n\n{}".format( rel_path, "\n".join("* {}".format(lint) for lint in lints) diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index cbaded859..21f337606 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -1997,7 +1997,7 @@ def test_lint_wheels(tmp_path, yaml_block, annotation): assert any(expected_message in hint for hint in hints) -class TestLintifyForgeYamlHintExtraFields(unittest.TestCase): +class TestLintifyForgeYamlHintExtraFields: def test_extra_build_platforms_platform(self): forge_yml = { "build_platform": { @@ -2008,11 +2008,9 @@ def test_extra_build_platforms_platform(self): hints = linter._forge_yaml_hint_extra_fields(forge_yml) - self.assertEquals(len(hints), 1) + assert len(hints) == 1 - self.assertIn( - "Unexpected key build_platform.UNKNOWN_PLATFORM", hints[0] - ) + assert "Unexpected key build_platform.UNKNOWN_PLATFORM" in hints[0] def test_extra_os_version_platform(self): forge_yml = { @@ -2023,9 +2021,9 @@ def test_extra_os_version_platform(self): hints = linter._forge_yaml_hint_extra_fields(forge_yml) - self.assertEquals(len(hints), 1) + assert len(hints) == 1 - self.assertIn("Unexpected key os_version.UNKNOWN_PLATFORM_2", hints[0]) + assert "Unexpected key os_version.UNKNOWN_PLATFORM_2" in hints[0] def test_extra_provider_platform(self): forge_yml = { @@ -2037,9 +2035,36 @@ def test_extra_provider_platform(self): hints = linter._forge_yaml_hint_extra_fields(forge_yml) - self.assertEquals(len(hints), 1) + assert len(hints) == 1 + + assert "Unexpected key provider.UNKNOWN_PLATFORM_3" in hints[0] + + @pytest.mark.parametrize( + "top_field", ["settings_linux", "settings_osx", "settings_win"] + ) + def test_extra_azure_runner_settings_no_hint(self, top_field: str): + forge_yml = { + "azure": { + top_field: { + "EXTRA_FIELD": "EXTRA_VALUE", + } + } + } + + hints = linter._forge_yaml_hint_extra_fields(forge_yml) + + assert len(hints) == 0 + + def test_extra_conda_build_config_no_hint(self): + forge_yml = { + "conda_build": { + "EXTRA_FIELD": "EXTRA_VALUE", + } + } + + hints = linter._forge_yaml_hint_extra_fields(forge_yml) - self.assertIn("Unexpected key provider.UNKNOWN_PLATFORM_3", hints[0]) + assert len(hints) == 0 if __name__ == "__main__": From 8a429ed4f1def8a2aa614364af3309539b5d5b4e Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 13 May 2024 16:40:29 +0200 Subject: [PATCH 6/8] make extra fields hint configurable don't warn for AzureRunnerSettings and CondaBuildConfig --- conda_smithy/data/conda-forge.json | 3 +++ conda_smithy/lint_recipe.py | 10 +++++++--- conda_smithy/schema.py | 22 ++++++++++++++++++++-- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/conda_smithy/data/conda-forge.json b/conda_smithy/data/conda-forge.json index 47916c021..d0c17797d 100644 --- a/conda_smithy/data/conda-forge.json +++ b/conda_smithy/data/conda-forge.json @@ -838,6 +838,7 @@ "type": "object" }, "build_platform": { + "additionalProperties": true, "properties": { "emscripten_wasm32": { "anyOf": [ @@ -1042,6 +1043,7 @@ "type": "object" }, "os_version": { + "additionalProperties": true, "properties": { "linux_32": { "anyOf": [ @@ -1183,6 +1185,7 @@ "type": "object" }, "provider": { + "additionalProperties": true, "properties": { "linux": { "anyOf": [ diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 0ba65a432..cb3181b6a 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -5,7 +5,7 @@ from pydantic import BaseModel -from conda_smithy.schema import ConfigModel +from conda_smithy.schema import ConfigModel, NoExtraFieldsHint str_type = str @@ -163,8 +163,12 @@ def _forge_yaml_hint_extra_fields(forge_yaml: dict) -> List[str]: hints = [] def _find_extra_fields(model: BaseModel, prefix=""): - for extra_field in (model.__pydantic_extra__ or {}).keys(): - hints.append(f"Unexpected key {prefix + extra_field}") + if not ( + isinstance(model, NoExtraFieldsHint) + and not model.HINT_EXTRA_FIELDS + ): + for extra_field in (model.__pydantic_extra__ or {}).keys(): + hints.append(f"Unexpected key {prefix + extra_field}") for field, value in model: if isinstance(value, BaseModel): diff --git a/conda_smithy/schema.py b/conda_smithy/schema.py index 568d5feb8..d241d3013 100644 --- a/conda_smithy/schema.py +++ b/conda_smithy/schema.py @@ -22,6 +22,15 @@ CONDA_FORGE_YAML_SCHEMA_FILE, ) +""" +Note: By default, we generate hints about additional fields the user added to the model +if extra="allow" is set. This can be disabled by inheriting from the NoExtraFieldsHint class +next to BaseModel. + +If adding new fields, you should decide between extra="forbid" and extra="allow", since +extra="ignore" (the default) will not generate hints about additional fields. +""" + class Nullable(Enum): """Created to avoid issue with schema validation of null values in lists or dicts.""" @@ -29,6 +38,15 @@ class Nullable(Enum): null = None +class NoExtraFieldsHint: + """ + Inherit from this class next to BaseModel to disable hinting about extra fields, even + if the model has `ConfigDict(extra="allow")`. + """ + + HINT_EXTRA_FIELDS = False + + ############################################# ######## Choices (Enum/Literals) definitions ######### ############################################# @@ -92,7 +110,7 @@ class BotConfigVersionUpdatesSourcesChoice(StrEnum): ############################################## -class AzureRunnerSettings(BaseModel): +class AzureRunnerSettings(BaseModel, NoExtraFieldsHint): """This is the settings for runners.""" model_config: ConfigDict = ConfigDict(extra="allow") @@ -383,7 +401,7 @@ class BotConfig(BaseModel): ) -class CondaBuildConfig(BaseModel): +class CondaBuildConfig(BaseModel, NoExtraFieldsHint): model_config: ConfigDict = ConfigDict(extra="allow") pkg_format: Optional[Literal["tar", 1, 2, "1", "2"]] = Field( From 6c0a9dbac6f6cc153b31bda4cde35e37d7df5dfc Mon Sep 17 00:00:00 2001 From: "H. Vetinari" Date: Fri, 22 Nov 2024 00:27:54 +1100 Subject: [PATCH 7/8] fix inaccurate tests --- tests/test_lint_recipe.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index acd8f7cdd..1249f99da 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -2459,8 +2459,8 @@ def test_lint_duplicate_cfyml(): fh.write( textwrap.dedent( """ - blah: 1 - blah: 2 + channel_priority: flexible + channel_priority: strict """ ) ) @@ -2472,7 +2472,7 @@ def test_lint_duplicate_cfyml(): fh.write( textwrap.dedent( """ - blah: 1 + channel_priority: flexible """ ) ) From bfc6a4b06cf0bb680f68ef876531633acc034309 Mon Sep 17 00:00:00 2001 From: "H. Vetinari" Date: Fri, 22 Nov 2024 00:40:53 +1100 Subject: [PATCH 8/8] appease linters --- conda_smithy/lint_recipe.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index fb6db7a3d..54076f7a3 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -23,7 +23,6 @@ from rattler_build_conda_compat import loader as rattler_loader from ruamel.yaml.constructor import DuplicateKeyError -from conda_smithy.schema import ConfigModel, NoExtraFieldsHint from conda_smithy.configure_feedstock import _read_forge_config from conda_smithy.linter import conda_recipe_v1_linter from conda_smithy.linter.hints import ( @@ -76,6 +75,7 @@ get_section, load_linter_toml_metdata, ) +from conda_smithy.schema import ConfigModel, NoExtraFieldsHint from conda_smithy.utils import get_yaml, render_meta_yaml from conda_smithy.validate_schema import validate_json_schema @@ -130,7 +130,9 @@ def _get_forge_yaml(recipe_dir: Optional[str] = None) -> dict: return forge_yaml -def lintify_forge_yaml(recipe_dir: Optional[str] = None) -> (list[str], list[str]): +def lintify_forge_yaml( + recipe_dir: Optional[str] = None, +) -> (list[str], list[str]): forge_yaml = _get_forge_yaml(recipe_dir) # This is where we validate against the jsonschema and execute our custom validators. json_lints, json_hints = validate_json_schema(forge_yaml)