SNOW-878611 renaming and aliasing CONFIG_PARSER to CONFIG_MANAGER (#1680)

Co-authored-by: Barry Warsaw <[email protected]>

Author: sfc-gh-mkeller

DESCRIPTION.md

@@ -8,6 +8,10 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
 
 # Release Notes
 
+- v3.1.2(TBD)
+
+  - Made the ``parser`` -> ``manager`` renaming more consistent in ``snowflake.connector.config_manager`` module.
+
 - v3.1.1(August 28,2023)
 
   - Fixed a bug in retry logic for okta authentication to refresh token.
@@ -16,7 +20,6 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
   - Cherry-picked https://github.com/urllib3/urllib3/commit/fd2759aa16b12b33298900c77d29b3813c6582de onto vendored urllib3 (v1.26.15) to enable enforce_content_length by default.
   - Fixed a bug in tag generation of OOB telemetry event.
 
-
 - v3.1.0(July 31,2023)
 
   - Added a feature that lets you add connection definitions to the `connections.toml` configuration file. A connection definition refers to a collection of connection parameters, for example, if you wanted to define a connection named `prod``:

src/snowflake/connector/config_manager.py

@@ -8,6 +8,7 @@
 import logging
 import os
 import stat
+import warnings
 from collections.abc import Iterable
 from operator import methodcaller
 from pathlib import Path
@@ -46,9 +47,11 @@ class ConfigSlice(NamedTuple):
 class ConfigOption:
     """ConfigOption represents a flag/setting.
 
-    The class knows how to read the value out of all sources and implements
+    This class knows how to read the value out of all different sources and implements
     order of precedence between them.
 
+    It also provides value parsing and verification.
+
     Attributes:
         name: Name of this ConfigOption.
         parse_str: A function that can turn str to the desired type, useful
@@ -57,14 +60,14 @@ class ConfigOption:
           this option.
         env_name: Environmental variable value should be read from, if not
           supplied, we'll construct this. False disables reading from
-          environmental variable, None uses the auto generated variable name
+          environmental variables, None uses the auto generated variable name
           and explicitly provided string overwrites the default one.
-        _root_manager: Reference to the root parser. Used to efficiently
+        _root_manager: Reference to the root manager. Used to efficiently
           refer to cached config file. Is supplied by the parent
           ConfigManager.
         _nest_path: The names of the ConfigManagers that this option is
-          nested in. Used be able to efficiently resolve where to grab
-          value out of configuration file and construct environment
+          nested in. Used to be able to efficiently resolve where to retrieve
+          value out of the configuration file and construct environment
           variable name. This is supplied by the parent ConfigManager.
     """
 
@@ -78,14 +81,17 @@ def __init__(
         _root_manager: ConfigManager | None = None,
         _nest_path: list[str] | None,
     ) -> None:
-        """Create a config option that can read values from different locations.
+        """Create a config option that can read values from different sources.
 
         Args:
             name: Name to assign to this ConfigOption.
-            parse_str: String parser function for this ConfigOption.
-            choices: List of possible values for this ConfigOption.
+            parse_str: String parser function for this instance.
+            choices: List of possible values for this instance.
             env_name: Environmental variable name value should be read from.
-            _root_manager: Reference to the root parser. Should be supplied by
+              Providing a string will use that environment variable, False disables
+              reading value from environmental variables and the default None generates
+              an environmental variable name for it using the _nest_path and name.
+            _root_manager: Reference to the root manager. Should be supplied by
               the parent ConfigManager.
             _nest_path: The names of the ConfigManagers that this option is
               nested in. This is supplied by the parent ConfigManager.
@@ -155,7 +161,11 @@ def _get_env(self) -> tuple[bool, str | _T | None]:
         return True, loaded_var
 
     def _get_config(self) -> Any:
-        """Get value from the cached config file."""
+        """Get value from the cached config file if possible.
+
+        Since this is the last resource for retrieving the value it raises
+        a MissingConfigOptionError if it's unable to find this option.
+        """
         if (
             self._root_manager.conf_file_cache is None
             and self._root_manager.file_path is not None
@@ -164,7 +174,7 @@ def _get_config(self) -> Any:
         e = self._root_manager.conf_file_cache
         if e is None:
             raise ConfigManagerError(
-                f"Root parser '{self._root_manager.name}' is missing file_path",
+                f"Root manager '{self._root_manager.name}' is missing file_path",
             )
         for k in self._nest_path[1:]:
             try:
@@ -183,13 +193,21 @@ def _get_config(self) -> Any:
 
 
 class ConfigManager:
-    """Read TOML configuration file with managed multi-source precedence.
+    """Read a TOML configuration file with managed multi-source precedence.
+
+    Note that multi-source precedence is actually implemented by ConfigOption.
+    This is done to make sure that special handling can be done for special options.
+    As an example, think of not allowing to provide passwords by command line arguments.
+
+    This class is updatable at run-time, allowing other libraries to add their
+    own configuration options and sub-managers before resolution.
 
-     This class is updatable at run-time, allowing other libraries to add their
-    own configuration options and sub-parsers. Sub-parsers allow
-     options groups to exist, e.g. the group "snowflake.cli.output" could have
-     2 options in it: debug (boolean flag) and format (a string like "json", or
-     "csv").
+    This class can simply be thought of as nestable containers for ConfigOptions.
+    It holds extra information necessary for efficient nesting purposes.
+
+    Sub-managers allow option groups to exist, e.g. the group "snowflake.cli.output"
+    could have 2 options in it: debug (boolean flag) and format (a string like "json",
+    or "csv").
 
     When a ConfigManager tries to retrieve ConfigOptions' value the _root_manager
     will read and cache the TOML file from the file it's pointing at, afterwards
@@ -200,14 +218,20 @@ class ConfigManager:
           useful error messages.
         file_path: Path to the file where this and all child ConfigManagers
           should read their values out of. Can be omitted for all child
-          parsers.
+          managers. Root manager could also miss this value, but this will
+          result in an exception when a value is read that isn't available from
+          a preceding config source.
         conf_file_cache: Cache to store what we read from the TOML file.
-        _sub_parsers: List of ConfigManagers that are nested under us.
-        _options: List of ConfigOptions that are our children.
-        _root_manager: Reference to root parser. Used to efficiently propagate to
+        _sub_managers: List of ConfigManagers that are nested under the current manager.
+        _sub_parsers: Alias for the old name of _sub_managers in the first release, please use
+          the new name now, as this might get deprecated in the future.
+        _options: List of ConfigOptions that are under the current manager.
+        _root_manager: Reference to the root manager. Used to efficiently propagate to
           child options.
-        _nest_path: The names of the ConfigManagers that this parser is nested
+        _nest_path: The names of the ConfigManagers that this manager is nested
           under. Used to efficiently propagate to child options.
+        _slices: List of config slices, where optional sections could be read from.
+          Note that this feature might become deprecated soon.
     """
 
     def __init__(
@@ -217,37 +241,54 @@ def __init__(
         file_path: Path | None = None,
         _slices: list[ConfigSlice] | None = None,
     ):
-        """Create a new ConfigManager.
+        """Creates a new ConfigManager.
 
         Args:
             name: Name of this ConfigManager.
-            file_path: File this parser should read values from. Can be omitted
-              for all child parsers.
+            file_path: File this manager should read values from. Can be omitted
+              for all child managers.
+            _slices: List of ConfigSlices to consider. A configuration file's slice is a
+              section that can optionally reside in a different file. Note that this
+              feature might get deprecated soon.
         """
         if _slices is None:
             _slices = list()
         self.name = name
         self.file_path = file_path
         self._slices = _slices
-        # Objects holding subparsers and options
+        # Objects holding sub-managers and options
         self._options: dict[str, ConfigOption] = dict()
-        self._sub_parsers: dict[str, ConfigManager] = dict()
+        self._sub_managers: dict[str, ConfigManager] = dict()
         # Dictionary to cache read in config file
         self.conf_file_cache: tomlkit.TOMLDocument | None = None
         # Information necessary to be able to nest elements
         #  and add options in O(1)
         self._root_manager: ConfigManager = self
         self._nest_path = [name]
 
+    @property
+    def _sub_parsers(self) -> dict[str, ConfigManager]:
+        """
+        Alias for the old name of ``_sub_managers``.
+
+        This used to be the original name  in the first release, please use the
+        new name, as this might get deprecated in the future.
+        """
+        warnings.warn(
+            "_sub_parsers has been deprecated, use _sub_managers instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self._sub_managers
+
     def read_config(
         self,
     ) -> None:
-        """Read and cache config file.
+        """Read and cache config file contents.
 
-        This function should be called if the ConfigManager's cache is outdated.
-        Maybe in the case when we want to replace the file_path assigned to a
-        ConfigManager, or if one's doing development and are interactively
-        adding new options to their configuration files.
+        This function should be explicitly called if the ConfigManager's cache is
+        outdated. Most likely when someone's doing development and are interactively
+        adding new options to their configuration file.
         """
         if self.file_path is None:
             raise ConfigManagerError(
@@ -297,10 +338,10 @@ def add_option(
         option_cls: type[ConfigOption] = ConfigOption,
         **kwargs,
     ) -> None:
-        """Add an ConfigOption to this ConfigManager.
+        """Add a ConfigOption to this ConfigManager.
 
         Args:
-            option_cls: The class that should be instantiated. This is class
+            option_cls: The class that should be instantiated. This class
               should be a child class of ConfigOption. Mainly useful for cases
               where the default ConfigOption needs to be extended, for example
               if a new configuration option source needs to be supported.
@@ -314,17 +355,17 @@ def add_option(
         self._options[new_option.name] = new_option
 
     def _check_child_conflict(self, name: str) -> None:
-        """Check if a sub-parser, or ConfigOption conflicts with given name.
+        """Check if a sub-manager, or ConfigOption conflicts with given name.
 
         Args:
             name: Name to check against children.
         """
-        if name in (self._options.keys() | self._sub_parsers.keys()):
+        if name in (self._options.keys() | self._sub_managers.keys()):
             raise ConfigManagerError(
-                f"'{name}' subparser, or option conflicts with a child element of '{self.name}'"
+                f"'{name}' sub-manager, or option conflicts with a child element of '{self.name}'"
             )
 
-    def add_subparser(self, new_child: ConfigManager) -> None:
+    def add_submanager(self, new_child: ConfigManager) -> None:
         """Nest another ConfigManager under this one.
 
         This function recursively updates _nest_path and _root_manager of all
@@ -334,44 +375,52 @@ def add_subparser(self, new_child: ConfigManager) -> None:
             new_child: The ConfigManager to be nested under the current one.
         Notes:
             We currently don't support re-nesting a ConfigManager. Only nest a
-            parser under another one once.
+            manager under another one once.
         """
         self._check_child_conflict(new_child.name)
-        self._sub_parsers[new_child.name] = new_child
+        self._sub_managers[new_child.name] = new_child
 
         def _root_setter_helper(node: ConfigManager):
             # Deal with ConfigManagers
             node._root_manager = self._root_manager
             node._nest_path = self._nest_path + node._nest_path
-            for sub_parser in node._sub_parsers.values():
-                _root_setter_helper(sub_parser)
+            for sub_manager in node._sub_managers.values():
+                _root_setter_helper(sub_manager)
             # Deal with ConfigOptions
             for option in node._options.values():
                 option._root_manager = self._root_manager
                 option._nest_path = self._nest_path + option._nest_path
 
         _root_setter_helper(new_child)
 
+    def add_subparser(self, *args, **kwargs) -> None:
+        warnings.warn(
+            "add_subparser has been deprecated, use add_submanager instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.add_submanager(*args, **kwargs)
+
     def __getitem__(self, name: str) -> ConfigOption | ConfigManager:
-        """Get either sub-parser, or option in this parser with name.
+        """Get either sub-manager, or option in this manager with name.
 
-        If option is retrieved, we call get() on it to return its value instead.
+        If an option is retrieved, we call get() on it to return its value instead.
 
         Args:
             name: Name to retrieve.
         """
         if name in self._options:
             return self._options[name].value()
-        if name not in self._sub_parsers:
+        if name not in self._sub_managers:
             raise ConfigSourceError(
                 "No ConfigManager, or ConfigOption can be found"
                 f" with the name '{name}'"
             )
-        return self._sub_parsers[name]
+        return self._sub_managers[name]
 
 
-CONFIG_PARSER = ConfigManager(
-    name="CONFIG_PARSER",
+CONFIG_MANAGER = ConfigManager(
+    name="CONFIG_MANAGER",
     file_path=CONFIG_FILE,
     _slices=[
         ConfigSlice(  # Optional connections file to read in connections from
@@ -383,13 +432,26 @@ def __getitem__(self, name: str) -> ConfigOption | ConfigManager:
         ),
     ],
 )
-CONFIG_PARSER.add_option(
+CONFIG_MANAGER.add_option(
     name="connections",
     parse_str=tomlkit.parse,
 )
 
-__all__ = [
+
+def __getattr__(name):
+    if name == "CONFIG_PARSER":
+        warnings.warn(
+            "CONFIG_PARSER has been deprecated, use CONFIG_MANAGER instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return CONFIG_MANAGER
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
+__all__ = [  # noqa: F822
     "ConfigOption",
     "ConfigManager",
+    "CONFIG_MANAGER",
     "CONFIG_PARSER",
 ]

src/snowflake/connector/connection.py

@@ -43,7 +43,7 @@
 from .auth.idtoken import AuthByIdToken
 from .bind_upload_agent import BindUploadError
 from .compat import IS_LINUX, IS_WINDOWS, quote, urlencode
-from .config_manager import CONFIG_PARSER
+from .config_manager import CONFIG_MANAGER
 from .connection_diagnostic import ConnectionDiagnostic
 from .constants import (
     ENV_VAR_PARTNER,
@@ -336,13 +336,13 @@ def __init__(
         self.query_context_cache_size = 5
         if connections_file_path is not None:
             # Change config file path and force update cache
-            for i, s in enumerate(CONFIG_PARSER._slices):
+            for i, s in enumerate(CONFIG_MANAGER._slices):
                 if s.section == "connections":
-                    CONFIG_PARSER._slices[i] = s._replace(path=connections_file_path)
-                    CONFIG_PARSER.read_config()
+                    CONFIG_MANAGER._slices[i] = s._replace(path=connections_file_path)
+                    CONFIG_MANAGER.read_config()
                     break
         if connection_name is not None:
-            connections = CONFIG_PARSER["connections"]
+            connections = CONFIG_MANAGER["connections"]
             if connection_name not in connections:
                 raise Error(
                     f"Invalid connection_name '{connection_name}',"

src/snowflake/connector/errors.py

@@ -620,7 +620,7 @@ class MissingConfigOptionError(ConfigSourceError):
 
 
 class ConfigManagerError(Error):
-    """Configuration parser related errors.
+    """Configuration manager related errors.
 
-    These mean that ConfigManager is misused by a developer.
+    This means that ConfigManager is misused by a developer.
     """