Abstract SamConfig

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,

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}")

samcli/lib/config/exceptions.py

@@ -5,3 +5,7 @@
 
 class SamConfigVersionException(Exception):
     pass
+
+
+class SamConfigFileReadException(Exception):
+    pass

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,30 +187,29 @@ 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):
         """
         Write the data back to file
 
         Raises
         ------
-        tomlkit.exceptions.TOMLKitError
+        SamConfigFileReadException
             If the data is invalid
 
         """
         self._write()
+        print(self.document)
 
     def sanity_check(self):
         """
         Sanity check the contents of samconfig
         """
         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)

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 (

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,15 +66,15 @@ 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):
         config_dir = tempfile.gettempdir()
         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)