diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 47d412511e..07dbf3f668 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -280,6 +280,9 @@ def run(self): self._handle_build_post_processing(builder, self._build_result) + for f in self.get_resources_to_build().functions: + EventTracker.track_event("BuildFunctionRuntime", f.runtime) + click.secho("\nBuild Succeeded", fg="green") # try to use relpath so the command is easier to understand, however, diff --git a/samcli/commands/deploy/guided_config.py b/samcli/commands/deploy/guided_config.py index b9d8ea59b5..10b80189db 100644 --- a/samcli/commands/deploy/guided_config.py +++ b/samcli/commands/deploy/guided_config.py @@ -7,6 +7,7 @@ from samcli.cli.context import get_cmd_names from samcli.commands.deploy.exceptions import GuidedDeployFailedError +from samcli.lib.config.exceptions import SamConfigFileReadException from samcli.lib.config.samconfig import DEFAULT_CONFIG_FILE_NAME, DEFAULT_ENV, SamConfig @@ -26,13 +27,17 @@ def get_config_ctx(self, config_file=None): return ctx, samconfig def read_config_showcase(self, config_file=None): - _, samconfig = self.get_config_ctx(config_file) - - status = "Found" if samconfig.exists() else "Not found" msg = ( "Syntax invalid in samconfig.toml; save values " "through sam deploy --guided to overwrite file with a valid set of values." ) + try: + _, samconfig = self.get_config_ctx(config_file) + except SamConfigFileReadException: + raise GuidedDeployFailedError(msg) + + status = "Found" if samconfig.exists() else "Not found" + config_sanity = samconfig.sanity_check() click.secho("\nConfiguring SAM deploy\n======================", fg="yellow") click.echo(f"\n\tLooking for config file [{config_file}] : {status}") diff --git a/samcli/lib/config/exceptions.py b/samcli/lib/config/exceptions.py index 50297ce722..6bf993de9b 100644 --- a/samcli/lib/config/exceptions.py +++ b/samcli/lib/config/exceptions.py @@ -5,3 +5,7 @@ class SamConfigVersionException(Exception): pass + + +class SamConfigFileReadException(Exception): + pass diff --git a/samcli/lib/config/samconfig.py b/samcli/lib/config/samconfig.py index e48e53d625..441754e014 100644 --- a/samcli/lib/config/samconfig.py +++ b/samcli/lib/config/samconfig.py @@ -4,12 +4,13 @@ import logging import os +from abc import ABC, abstractmethod from pathlib import Path from typing import Any, Iterable import tomlkit -from samcli.lib.config.exceptions import SamConfigVersionException +from samcli.lib.config.exceptions import SamConfigFileReadException, SamConfigVersionException from samcli.lib.config.version import SAM_CONFIG_VERSION, VERSION_KEY LOG = logging.getLogger(__name__) @@ -20,12 +21,56 @@ DEFAULT_GLOBAL_CMDNAME = "global" +class FileManager(ABC): + """ + Abstract class to be overridden by file managers for specific file extensions. + """ + + @staticmethod + @abstractmethod + def read(filepath: Path) -> dict: + pass + + @staticmethod + @abstractmethod + def write(document: dict, filepath: Path): + pass + + +class TomlFileManager(FileManager): + """ + Static class to read and write toml files. + """ + + @staticmethod + def read(filepath: Path) -> dict: + document: dict = {} + try: + txt = filepath.read_text() + document = dict(tomlkit.loads(txt)) + except OSError: + document = {} + except tomlkit.exceptions.TOMLKitError as e: + raise SamConfigFileReadException(e) + + return document + + @staticmethod + def write(document: dict, filepath: Path): + if not document: + return + + filepath.write_text(tomlkit.dumps(document)) + + class SamConfig: """ - Class to interface with `samconfig.toml` file. + Class to represent `samconfig` config options. """ - document = None + FILE_MANAGER_MAPPER: dict = { + "toml": TomlFileManager, + } def __init__(self, config_dir, filename=None): """ @@ -39,11 +84,15 @@ def __init__(self, config_dir, filename=None): Optional. Name of the configuration file. It is recommended to stick with default so in the future we could automatically support auto-resolving multiple config files within same directory. """ + self.document = {} self.filepath = Path(config_dir, filename or DEFAULT_CONFIG_FILE_NAME) + self.file_manager = self.FILE_MANAGER_MAPPER.get(self.filepath.suffix[1:], TomlFileManager) # default to TOML + if not self.file_manager: + LOG.warning(f"The config file extension '{self.filepath.suffix[1:]}' is not supported. TOML will be used.") + self._read() def get_stage_configuration_names(self): - self._read() - if isinstance(self.document, dict): + if self.document: return [stage for stage, value in self.document.items() if isinstance(value, dict)] return [] @@ -70,22 +119,21 @@ def get_all(self, cmd_names, section, env=DEFAULT_ENV): KeyError If the config file does *not* have the specific section - tomlkit.exceptions.TOMLKitError + SamConfigFileReadException If the configuration file is invalid """ env = env or DEFAULT_ENV - self._read() - if isinstance(self.document, dict): - toml_content = self.document.get(env, {}) - params = toml_content.get(self._to_key(cmd_names), {}).get(section, {}) - if DEFAULT_GLOBAL_CMDNAME in toml_content: - global_params = toml_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}) - global_params.update(params.copy()) - params = global_params.copy() - return params - return {} + self.document = self.file_manager.read(self.filepath) + + config_content = self.document.get(env, {}) + params = config_content.get(self._to_key(cmd_names), {}).get(section, {}) + if DEFAULT_GLOBAL_CMDNAME in config_content: + global_params = config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}) + global_params.update(params.copy()) + params = global_params.copy() + return params def put(self, cmd_names, section, key, value, env=DEFAULT_ENV): """ @@ -102,20 +150,15 @@ def put(self, cmd_names, section, key, value, env=DEFAULT_ENV): key : str Key to write the data under value : Any - Value to write. Could be any of the supported TOML types. + Value to write. Could be any of the supported types. env : str Optional, Name of the environment Raises ------ - tomlkit.exceptions.TOMLKitError + SamConfigFileReadException If the data is invalid """ - - if self.document is None: - # Checking for None here since a TOMLDocument can include a - # 'body' property but still be falsy without a 'value' property - self._read() # Empty document prepare the initial structure. # self.document is a nested dict, we need to check each layer and add new tables, otherwise duplicated key # in parent layer will override the whole child layer @@ -144,10 +187,8 @@ def put_comment(self, comment): comment: str A comment to write to the samconfg file """ - if self.document is None: - self._read() - self.document.add(tomlkit.comment(comment)) + self.document.update({"__comment__": comment}) def flush(self): """ @@ -155,11 +196,12 @@ def flush(self): Raises ------ - tomlkit.exceptions.TOMLKitError + SamConfigFileReadException If the data is invalid """ self._write() + print(self.document) def sanity_check(self): """ @@ -167,7 +209,7 @@ def sanity_check(self): """ try: self._read() - except tomlkit.exceptions.TOMLKitError: + except SamConfigFileReadException: return False else: return True @@ -195,14 +237,8 @@ def config_dir(template_file_path=None): def _read(self): if not self.document: - try: - txt = self.filepath.read_text() - self.document = tomlkit.loads(txt) - self._version_sanity_check(self._version()) - except OSError: - self.document = tomlkit.document() - - if self.document.body: + self.document = self.file_manager.read(self.filepath) + if self.document: self._version_sanity_check(self._version()) return self.document @@ -213,12 +249,9 @@ def _write(self): self._ensure_exists() current_version = self._version() if self._version() else SAM_CONFIG_VERSION - try: - self.document.add(VERSION_KEY, current_version) - except tomlkit.exceptions.KeyAlreadyPresent: - # NOTE(TheSriram): Do not attempt to re-write an existing version - pass - self.filepath.write_text(tomlkit.dumps(self.document)) + self.document.update({VERSION_KEY: current_version}) + + self.file_manager.write(self.document, self.filepath) def _version(self): return self.document.get(VERSION_KEY, None) diff --git a/samcli/lib/sync/sync_flow_executor.py b/samcli/lib/sync/sync_flow_executor.py index d6f712cfea..fd11222fcc 100644 --- a/samcli/lib/sync/sync_flow_executor.py +++ b/samcli/lib/sync/sync_flow_executor.py @@ -9,6 +9,7 @@ from uuid import uuid4 from botocore.exceptions import ClientError +from samcli.lib.telemetry.event import EventName, EventTracker, EventType from samcli.lib.providers.exceptions import MissingLocalDefinition from samcli.lib.sync.exceptions import ( diff --git a/tests/unit/cli/test_cli_config_file.py b/tests/unit/cli/test_cli_config_file.py index 606e0a004e..8177b3b9da 100644 --- a/tests/unit/cli/test_cli_config_file.py +++ b/tests/unit/cli/test_cli_config_file.py @@ -5,9 +5,12 @@ from unittest import TestCase, skipIf from unittest.mock import MagicMock, patch +import tomlkit + from samcli.commands.exceptions import ConfigException from samcli.cli.cli_config_file import TomlProvider, configuration_option, configuration_callback, get_ctx_defaults -from samcli.lib.config.samconfig import DEFAULT_ENV +from samcli.lib.config.exceptions import SamConfigFileReadException, SamConfigVersionException +from samcli.lib.config.samconfig import DEFAULT_ENV, TomlFileManager from tests.testing_utils import IS_WINDOWS @@ -40,14 +43,14 @@ def test_toml_valid_with_no_version(self): config_dir = tempfile.gettempdir() config_path = Path(config_dir, "samconfig.toml") config_path.write_text("[config_env.topic.parameters]\nword='clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigVersionException): TomlProvider(section=self.parameters)(config_path, self.config_env, [self.cmd_name]) def test_toml_valid_with_invalid_version(self): config_dir = tempfile.gettempdir() config_path = Path(config_dir, "samconfig.toml") config_path.write_text("version='abc'\n[config_env.topic.parameters]\nword='clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigVersionException): TomlProvider(section=self.parameters)(config_path, self.config_env, [self.cmd_name]) def test_toml_invalid_empty_dict(self): @@ -63,7 +66,7 @@ def test_toml_invalid_file_name(self): config_path.write_text("version=0.1\n[config_env.topic.parameters]\nword='clarity'\n") config_path_invalid = Path(config_dir, "samconfig.toml") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigFileReadException): self.toml_provider(config_path_invalid, self.config_env, [self.cmd_name]) def test_toml_invalid_syntax(self): @@ -71,7 +74,7 @@ def test_toml_invalid_syntax(self): config_path = Path(config_dir, "samconfig.toml") config_path.write_text("version=0.1\n[config_env.topic.parameters]\nword=_clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigFileReadException): self.toml_provider(config_path, self.config_env, [self.cmd_name]) @@ -231,3 +234,53 @@ def test_get_ctx_defaults_nested(self): get_ctx_defaults("intent-answer", provider, mock_context4, "default") provider.assert_called_with(None, "default", ["local", "generate-event", "alexa-skills-kit", "intent-answer"]) + + +class TestTomlFileManager(TestCase): + def test_read_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("version=0.1\n[config_env.topic1.parameters]\nword='clarity'\n") + self.assertEqual( + TomlFileManager.read(config_path), + {"version": 0.1, "config_env": {"topic1": {"parameters": {"word": "clarity"}}}}, + ) + + def test_read_toml_invalid_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("fake='not real'\nimproper toml file\n") + with self.assertRaises(SamConfigFileReadException): + TomlFileManager.read(config_path) + + def test_read_toml_file_path_not_valid(self): + config_dir = "path/that/doesnt/exist" + config_path = Path(config_dir, "samconfig.toml") + self.assertEqual(TomlFileManager.read(config_path), {}) + + def test_write_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + toml = {"version": 0.1, "config_env": {"topic2": {"parameters": {"word": "clarity"}}}} + + TomlFileManager.write(toml, config_path) + + txt = config_path.read_text() + self.assertIn("version = 0.1", txt) + self.assertIn("[config_env.topic2.parameters]", txt) + self.assertIn('word = "clarity"', txt) + + def test_dont_write_toml_if_empty(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("nothing to see here\n") + toml = {} + + TomlFileManager.write(toml, config_path) + + self.assertEqual(config_path.read_text(), "nothing to see here\n") + + def test_write_toml_bad_path(self): + config_path = Path("path/to/some", "file_that_doesnt_exist.toml") + with self.assertRaises(FileNotFoundError): + TomlFileManager.write({"some key": "some value"}, config_path) diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index 1ff1217138..6f69ae87eb 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -8,7 +8,6 @@ import tempfile from pathlib import Path from contextlib import contextmanager -from samcli.lib.config.samconfig import SamConfig, DEFAULT_ENV from click.testing import CliRunner @@ -16,6 +15,7 @@ from unittest.mock import patch, ANY import logging +from samcli.lib.config.samconfig import SamConfig, DEFAULT_ENV, TomlFileManager from samcli.lib.utils.packagetype import ZIP, IMAGE LOG = logging.getLogger() @@ -1245,6 +1245,24 @@ def test_secondary_option_name_template_validate(self, do_cli_mock): do_cli_mock.assert_called_with(ANY, str(Path(os.getcwd(), "mytemplate.yaml")), False) +class TestSamConfigFileManager(TestCase): + def test_file_manager_defaults_to_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig") + + samconfig = SamConfig(config_path, filename="samconfig") + + self.assertIs(samconfig.file_manager, TomlFileManager) + + def test_file_manager_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + + samconfig = SamConfig(config_path, filename="samconfig.toml") + + self.assertIs(samconfig.file_manager, TomlFileManager) + + @contextmanager def samconfig_parameters(cmd_names, config_dir=None, env=None, **kwargs): """ diff --git a/tests/unit/lib/samconfig/test_samconfig.py b/tests/unit/lib/samconfig/test_samconfig.py index 7a86e6f97d..b3a3a7e858 100644 --- a/tests/unit/lib/samconfig/test_samconfig.py +++ b/tests/unit/lib/samconfig/test_samconfig.py @@ -182,27 +182,27 @@ def test_check_sanity(self): def test_check_version_non_supported_type(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, "aadeff") + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: "aadeff"}) with self.assertRaises(SamConfigVersionException): self.samconfig.sanity_check() def test_check_version_no_version_exists(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) + self.samconfig.document.pop(VERSION_KEY) with self.assertRaises(SamConfigVersionException): self.samconfig.sanity_check() def test_check_version_float(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, 0.2) + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: 0.2}) self.samconfig.sanity_check() def test_write_config_file_non_standard_version(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, 0.2) + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: 0.2}) self.samconfig.put(cmd_names=["local", "start", "api"], section="parameters", key="skip_pull_image", value=True) self.samconfig.sanity_check() self.assertEqual(self.samconfig.document.get(VERSION_KEY), 0.2) diff --git a/tests/unit/lib/telemetry/test_metric.py b/tests/unit/lib/telemetry/test_metric.py index 22f695eebe..16f3b91b66 100644 --- a/tests/unit/lib/telemetry/test_metric.py +++ b/tests/unit/lib/telemetry/test_metric.py @@ -3,6 +3,7 @@ from unittest import TestCase from unittest.mock import patch, Mock, ANY, call +from samcli.lib.telemetry.event import EventTracker import pytest import click @@ -394,6 +395,18 @@ def test_context_contains_exception(self, expected_exception, expected_code, sen send_events_mock.assert_called() + @patch("samcli.lib.telemetry.event.EventTracker.send_events", return_value=None) + @patch("samcli.lib.telemetry.metric.Context") + def test_must_send_events(self, ContextMock, send_mock): + ContextMock.get_current_context.return_value = self.context_mock + + def real_fn(): + pass + + track_command(real_fn)() + + send_mock.assert_called() + class TestParameterCapture(TestCase): def setUp(self):