From 8624b08a28e18cfe7976ffe346befeaf0104ca48 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Mon, 24 Jul 2023 14:40:48 -0700 Subject: [PATCH 1/3] Fix generation logic --- schema/{schema.py => make_schema.py} | 21 +++++++++--- schema/samcli.json | 46 ++++++++++++++++++++------ tests/unit/schema/test_schema_logic.py | 23 ++++++------- 3 files changed, 62 insertions(+), 28 deletions(-) rename schema/{schema.py => make_schema.py} (92%) diff --git a/schema/schema.py b/schema/make_schema.py similarity index 92% rename from schema/schema.py rename to schema/make_schema.py index 122cf15e65..2645ef790c 100644 --- a/schema/schema.py +++ b/schema/make_schema.py @@ -20,6 +20,15 @@ PARAMS_TO_OMIT_DEFAULT_FIELD = [ "layer_cache_basedir" # sets default to root directory to that of machine the schema is generated on ] +EXCEPTION_OPTION_CLASS_MAPPER = { # for params without proper class names, map them uniquely + "parameter_overrides": ["array", "string"], + "image_repository": "string", + "image_repositories": "array", + "metadata": "string", + "signing_profiles": "string", + "tags": "array", + "parameter": "array", +} CHARS_TO_CLEAN = [ "\b", # backspaces "\u001b[0m", # ANSI start bold @@ -147,14 +156,16 @@ def format_param(param: click.core.Option) -> SamCliParameterSchema: """ if not param: raise SchemaGenerationException("Expected to format a parameter that doesn't exist") - if not param.type.name: - raise SchemaGenerationException(f"Parameter {param} passed without a type") + if not param.type.name and param.name not in EXCEPTION_OPTION_CLASS_MAPPER.keys(): + raise SchemaGenerationException(f"Parameter {param} passed without a type:\n{param.type}") param_type = param.type.name.lower() - formatted_param_type = "" + formatted_param_type: Any = None # NOTE: Params do not have explicit "string" type; either "text" or "path". # All choice options are from a set of strings. - if param_type in ["text", "path", "choice", "filename", "directory"]: + if param.name in EXCEPTION_OPTION_CLASS_MAPPER.keys(): + formatted_param_type = EXCEPTION_OPTION_CLASS_MAPPER[param.name] + elif param_type in ["text", "path", "choice", "filename", "directory"]: formatted_param_type = "string" elif param_type == "list": formatted_param_type = "array" @@ -165,7 +176,7 @@ def format_param(param: click.core.Option) -> SamCliParameterSchema: param.name or "", formatted_param_type, clean_text(param.help or ""), - items="string" if formatted_param_type == "array" else None, + items="string" if formatted_param_type == "array" or "array" in formatted_param_type else None, ) if param.default and param.name not in PARAMS_TO_OMIT_DEFAULT_FIELD: diff --git a/schema/samcli.json b/schema/samcli.json index 6bc0586be5..e1ddfd79b0 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -313,7 +313,10 @@ }, "parameter_overrides": { "title": "parameter_overrides", - "type": "string", + "type": [ + "array", + "string" + ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." }, "skip_pull_image": { @@ -396,7 +399,10 @@ }, "parameter_overrides": { "title": "parameter_overrides", - "type": "string", + "type": [ + "array", + "string" + ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." }, "debug_port": { @@ -548,7 +554,10 @@ }, "parameter_overrides": { "title": "parameter_overrides", - "type": "string", + "type": [ + "array", + "string" + ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." }, "debug_port": { @@ -723,7 +732,10 @@ }, "parameter_overrides": { "title": "parameter_overrides", - "type": "string", + "type": [ + "array", + "string" + ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." }, "debug_port": { @@ -871,8 +883,11 @@ }, "image_repositories": { "title": "image_repositories", - "type": "string", - "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times." + "type": "array", + "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.", + "items": { + "type": "string" + } }, "s3_prefix": { "title": "s3_prefix", @@ -1010,8 +1025,11 @@ }, "image_repositories": { "title": "image_repositories", - "type": "string", - "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times." + "type": "array", + "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.", + "items": { + "type": "string" + } }, "force_upload": { "title": "force_upload", @@ -1063,12 +1081,18 @@ }, "tags": { "title": "tags", - "type": "string", - "description": "List of tags to associate with the stack." + "type": "array", + "description": "List of tags to associate with the stack.", + "items": { + "type": "string" + } }, "parameter_overrides": { "title": "parameter_overrides", - "type": "string", + "type": [ + "array", + "string" + ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." }, "signing_profiles": { diff --git a/tests/unit/schema/test_schema_logic.py b/tests/unit/schema/test_schema_logic.py index 4fae6c5d55..fcee481515 100644 --- a/tests/unit/schema/test_schema_logic.py +++ b/tests/unit/schema/test_schema_logic.py @@ -1,11 +1,10 @@ from typing import List -from unittest.mock import MagicMock, Mock, patch -import click +from unittest.mock import MagicMock, patch from parameterized import parameterized from unittest import TestCase from schema.exceptions import SchemaGenerationException -from schema.schema import ( +from schema.make_schema import ( SamCliCommandSchema, SamCliParameterSchema, SchemaKeys, @@ -121,7 +120,7 @@ def test_param_formatted_throws_error_when_none(self): ("choice", SamCliParameterSchema("p_name", "string", default=["default", "value"], choices=["1", "2"])), ] ) - @patch("schema.schema.isinstance") + @patch("schema.make_schema.isinstance") def test_param_formatted_given_type(self, param_type, expected_param, isinstance_mock): mock_param = MagicMock() mock_param.name = "p_name" @@ -135,8 +134,8 @@ def test_param_formatted_given_type(self, param_type, expected_param, isinstance self.assertEqual(expected_param, formatted_param) - @patch("schema.schema.isinstance") - @patch("schema.schema.format_param") + @patch("schema.make_schema.isinstance") + @patch("schema.make_schema.format_param") def test_getting_params_from_cli_object(self, format_param_mock, isinstance_mock): mock_cli = MagicMock() mock_cli.params = [] @@ -154,8 +153,8 @@ def test_getting_params_from_cli_object(self, format_param_mock, isinstance_mock self.assertNotIn("config_file", params) self.assertNotIn(None, params) - @patch("schema.schema.importlib.import_module") - @patch("schema.schema.get_params_from_command") + @patch("schema.make_schema.importlib.import_module") + @patch("schema.make_schema.get_params_from_command") def test_command_structure_is_retrieved(self, get_params_mock, import_mock): mock_module = self._setup_mock_module() import_mock.side_effect = lambda _: mock_module @@ -165,9 +164,9 @@ def test_command_structure_is_retrieved(self, get_params_mock, import_mock): self._validate_retrieved_command_structure(commands) - @patch("schema.schema.importlib.import_module") - @patch("schema.schema.get_params_from_command") - @patch("schema.schema.isinstance") + @patch("schema.make_schema.importlib.import_module") + @patch("schema.make_schema.get_params_from_command") + @patch("schema.make_schema.isinstance") def test_command_structure_is_retrieved_from_group_cli(self, isinstance_mock, get_params_mock, import_mock): mock_module = self._setup_mock_module() mock_module.cli.commands = {} @@ -183,7 +182,7 @@ def test_command_structure_is_retrieved_from_group_cli(self, isinstance_mock, ge self._validate_retrieved_command_structure(commands) - @patch("schema.schema.retrieve_command_structure") + @patch("schema.make_schema.retrieve_command_structure") def test_schema_is_generated_properly(self, retrieve_commands_mock): def mock_retrieve_commands(package_name, counter=[0]): counter[0] += 1 From a98cdd01e5463dfac7769f9d75dd8c565107cdcd Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Mon, 24 Jul 2023 15:37:55 -0700 Subject: [PATCH 2/3] Simplify param type to list --- schema/make_schema.py | 26 +++++++++++++------------- schema/samcli.json | 25 ++++++++++++++++++++----- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/schema/make_schema.py b/schema/make_schema.py index 2645ef790c..2f6ae951b8 100644 --- a/schema/make_schema.py +++ b/schema/make_schema.py @@ -22,12 +22,12 @@ ] EXCEPTION_OPTION_CLASS_MAPPER = { # for params without proper class names, map them uniquely "parameter_overrides": ["array", "string"], - "image_repository": "string", - "image_repositories": "array", - "metadata": "string", - "signing_profiles": "string", - "tags": "array", - "parameter": "array", + "image_repository": ["string"], + "image_repositories": ["array"], + "metadata": ["string"], + "signing_profiles": ["string"], + "tags": ["array"], + "parameter": ["array"], } CHARS_TO_CLEAN = [ "\b", # backspaces @@ -160,23 +160,23 @@ def format_param(param: click.core.Option) -> SamCliParameterSchema: raise SchemaGenerationException(f"Parameter {param} passed without a type:\n{param.type}") param_type = param.type.name.lower() - formatted_param_type: Any = None + formatted_param_types = [] # NOTE: Params do not have explicit "string" type; either "text" or "path". # All choice options are from a set of strings. if param.name in EXCEPTION_OPTION_CLASS_MAPPER.keys(): - formatted_param_type = EXCEPTION_OPTION_CLASS_MAPPER[param.name] + formatted_param_types = EXCEPTION_OPTION_CLASS_MAPPER[param.name] elif param_type in ["text", "path", "choice", "filename", "directory"]: - formatted_param_type = "string" + formatted_param_types = ["string"] elif param_type == "list": - formatted_param_type = "array" + formatted_param_types = ["array"] else: - formatted_param_type = param_type or "string" + formatted_param_types = [param_type] or ["string"] formatted_param: SamCliParameterSchema = SamCliParameterSchema( param.name or "", - formatted_param_type, + formatted_param_types if len(formatted_param_types) > 1 else formatted_param_types[0], clean_text(param.help or ""), - items="string" if formatted_param_type == "array" or "array" in formatted_param_type else None, + items="string" if "array" in formatted_param_types else None, ) if param.default and param.name not in PARAMS_TO_OMIT_DEFAULT_FIELD: diff --git a/schema/samcli.json b/schema/samcli.json index e1ddfd79b0..2911bbac7e 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -317,7 +317,10 @@ "array", "string" ], - "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." + "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", + "items": { + "type": "string" + } }, "skip_pull_image": { "title": "skip_pull_image", @@ -403,7 +406,10 @@ "array", "string" ], - "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." + "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", + "items": { + "type": "string" + } }, "debug_port": { "title": "debug_port", @@ -558,7 +564,10 @@ "array", "string" ], - "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." + "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", + "items": { + "type": "string" + } }, "debug_port": { "title": "debug_port", @@ -736,7 +745,10 @@ "array", "string" ], - "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." + "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", + "items": { + "type": "string" + } }, "debug_port": { "title": "debug_port", @@ -1093,7 +1105,10 @@ "array", "string" ], - "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs." + "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", + "items": { + "type": "string" + } }, "signing_profiles": { "title": "signing_profiles", From b735548d589da92ad196bc48cb56f8d9ba72baa2 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Mon, 24 Jul 2023 15:55:45 -0700 Subject: [PATCH 3/3] Satisfy linter --- schema/make_schema.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/schema/make_schema.py b/schema/make_schema.py index 2f6ae951b8..19a561c1e6 100644 --- a/schema/make_schema.py +++ b/schema/make_schema.py @@ -5,7 +5,7 @@ import json from dataclasses import dataclass from enum import Enum -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Union import click @@ -52,7 +52,7 @@ class SamCliParameterSchema: """ name: str - type: str + type: Union[str, List[str]] description: str = "" default: Optional[Any] = None items: Optional[str] = None