Review: a few fixes
authorChristian Herdtweck <christian.herdtweck@intra2net.com>
Fri, 1 Apr 2022 14:41:42 +0000 (16:41 +0200)
committerChristian Herdtweck <christian.herdtweck@intra2net.com>
Tue, 5 Apr 2022 07:19:13 +0000 (09:19 +0200)
functional changes:
* get to run on python < 3.8
* add missing args for shadowed functions startswith/endswith
* some more

non-functional changes:
* add some doc strings (mostly arnied_api)
* wrap very long lines
* add some whitespace to make linter happier
* fix a few typos
* remove unnecessary "object" super class
* fix api documentation
* add diagram that Samir had created to api doc

.gitignore
doc/cnfvar-api-ascii-art.rst [new file with mode: 0644]
doc/conf.py
doc/index.rst
src/arnied_api.py
src/cnfvar/__init__.py
src/cnfvar/binary.py
src/cnfvar/model.py
src/cnfvar/store.py
src/text_helpers.py
test/cnfvar/test_store.py

index 3d19e45..b8336d5 100644 (file)
@@ -6,7 +6,10 @@ build/
 dist/
 MANIFEST
 _build/
-doc/
+doc/_build
+doc/src.rst
+doc/src.cnfline.rst
+doc/src.cnfvar.rst
 
 # PyDev
 .project
diff --git a/doc/cnfvar-api-ascii-art.rst b/doc/cnfvar-api-ascii-art.rst
new file mode 100644 (file)
index 0000000..9f49bb7
--- /dev/null
@@ -0,0 +1,30 @@
+Overview Diagram of cnfvar package
+==================================
+
+::
+
+  frontend    .    CRUD layer      .    backend drivers  .       backend
+              |                    |                     |
+              |                    |                     |      .-,(  ),-.
+      o       |                    |                     |   .-(          )-.
+     /|\      | .----------.       |            .---------->(   varlink API  )
+     / \      |/| CnfStore |       |            |        |   '-(          ).-'
+              / '----------'       |            |        |       '-.( ).-'
+             /|       |            |    .-------------.  |     |
+ .----------' |       |            |    | varlink API |  |     |
+ | CnfList |  |       '---------------->|  interface  |  |     |    _.-----._
+ '--.------.  |                    |    '-------------'  |     |  .-         -.
+    |.-----.\ |                    |                     |     '->|-_       _-|
+    || Cnf | \| .----------------. |                     |        |  ~-----~  |
+    |'-----'  \ | BinaryCnfStore | |                     |     .->|   cnfvar  |
+    |.-----.  |'-----------------' |                     |     |  `._backend_.'
+    || Cnf |  |         |          |    .------------.   |     |     "-----"
+    |'-----'  |         |          |    | subprocess |   |     |
+    |.-----.  |         '-------------->|  wrappers  |   |     _______
+    '| Cnf |  |                    |    '------------'   |    |get_cnf|
+     '-----'  |                    |           |         |    |set_cnf|
+              |                    |           '------------> |_______|
+              |                    |                     |    /:::::::/
+              |                    |                     |
+              '                    '                     '
+
index 97ef19d..ec74d6f 100644 (file)
@@ -55,7 +55,7 @@ master_doc = 'index'
 
 # General information about the project.
 project = 'pyi2ncommon'
-copyright = '2016-2018, Intra2net AG <info@intra2net.com>'
+copyright = 'Intra2net AG <info@intra2net.com>'
 author = 'Intra2net AG'
 
 # The version info for the project you're documenting, acts as replacement for
@@ -63,9 +63,9 @@ author = 'Intra2net AG'
 # built documents.
 #
 # The short X.Y version.
-version = '0'
+version = '1.6'
 # The full version, including alpha/beta/rc tags.
-release = '1'
+release = '7'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.
@@ -293,7 +293,7 @@ texinfo_documents = [
 
 
 # Example configuration for intersphinx: refer to the Python standard library.
-intersphinx_mapping = {'https://docs.python.org/3.3': None}
+intersphinx_mapping = {'https://docs.python.org/3': None}
 
 # show doc of class AND of constructor
 autoclass_content = 'class'  # 'both' not needed if special-members is used
@@ -307,8 +307,8 @@ autodoc_member_order = 'bysource'
 def skip_class_members(app, what, name, obj, skip, options):
     exclude = name in ('__weakref__', '__dict__', '__module__', '__doc__') \
         and what == 'class'
-    if exclude:
-        print('excluding {0}'.format(name))
+    #if exclude:
+    #    print('excluding {0}'.format(name))
     return skip or exclude
 
 def setup(app):
index 7c85760..ffd006d 100644 (file)
@@ -15,7 +15,7 @@ Contents:
 
    license.rst
    about_docu.rst
-   template
+   cnfvar-api-ascii-art.rst
 
 
 Modules
index 5d8b868..537375b 100644 (file)
@@ -24,6 +24,12 @@ arnied_api: wrappers around the arnied varlink API.
 Featuring
 - Arnied: stateless class with methods as exposed in the varlink API.
 
+For documentation of Exceptions, methods and their arguments, refer to arnied
+source code (arnied/client/arnieclient.hxx). For compatibility, argument names
+are the same here as they are there (defeating python naming rules).
+
+See package `cnfvar` for a higher-level interface to this functionality.
+
 .. codeauthor:: Intra2net
 """
 
@@ -44,15 +50,16 @@ ARNIED_SOCKET = "/var/intranator/arnied-varlink.sock"
 
 
 @dataclass
-class CnfVar(object):
+class CnfVar:
     name: str
     instance: int
     data: str
     deleted: bool
     children: typing.List['CnfVar']
 
+
 @dataclass
-class ChangeCnfVar(object):
+class ChangeCnfVar:
     chg_nr: int
     name: str
     instance: int
@@ -60,11 +67,13 @@ class ChangeCnfVar(object):
     result_type: int
     result_msg: str
 
+
 @dataclass
-class GetCnfQuery(object):
+class GetCnfQuery:
     name: str
     instance: typing.Optional[int] = None
 
+
 class ProgramStatus(Enum):
     Running = auto()
     Scheduled = auto()
@@ -76,19 +85,24 @@ class ProgramStatus(Enum):
 class IsQueueActiveRet(SimpleNamespace):
     active: bool
 
+
 class IsScheduledOrRunningRet(SimpleNamespace):
     status: ProgramStatus
     timestamp: typing.Optional[int]
 
+
 class GetCnfRet(SimpleNamespace):
     vars: typing.List[CnfVar]
 
+
 class SetCnfRet(SimpleNamespace):
     change_numbers: typing.List[int]
 
+
 class SetCommitCnf(SimpleNamespace):
     results: typing.List[ChangeCnfVar]
 
+
 class CommitCnfRet(SimpleNamespace):
     results: typing.List[ChangeCnfVar]
 
@@ -100,20 +114,24 @@ class InternalBridgeError(Exception):
     def __init__(self):
         super().__init__("An error occurred (InternalBridgeError)")
 
+
 class EmptyInputError(Exception):
     def __init__(self):
         super().__init__("An error occurred (EmptyInputError)")
 
+
 class BadCharError(Exception):
     def __init__(self, results: str) -> None:
         super().__init__(f"[BadCharError] Error in the arnied API (results={results})")
         self.results = results
 
+
 class ChildError(Exception):
     def __init__(self, results: str) -> None:
         super().__init__(f"[ChildError] Error in the arnied API (results={results})")
         self.results = results
 
+
 class CnfCommitError(Exception):
     def __init__(self, results: typing.List[ChangeCnfVar]) -> None:
         self.results = []
@@ -124,37 +142,50 @@ class CnfCommitError(Exception):
                 r = ChangeCnfVar(**r)
             self.results.append(r)
             msgs.append(f"{r.name},{r.instance}: \"{r.data}\": {r.result_msg}")
-        super().__init__("Error commiting cnfvars:\n" + "\n".join(msgs))
+        super().__init__("Error committing cnfvars:\n" + "\n".join(msgs))
+
 
 class NotFoundError(Exception):
     def __init__(self, results: str) -> None:
         super().__init__(f"[NotFoundError] Error in the arnied API (results={results})")
         self.results = results
 
+
 class SchedulerProgError(Exception):
     def __init__(self):
         super().__init__("An error occurred (SchedulerProgError)")
 
+
 class ShowerrVarDataErr(Exception):
     def __init__(self, msg_id: int) -> None:
         super().__init__(f"[ShowerrVarDataErr] Error in the arnied API (msg_id={msg_id})")
         self.msg_id = msg_id
 
+
 class ShowerrVarMissing(Exception):
     def __init__(self, params_count: int) -> None:
-        super().__init__(f"[ShowerrVarMissing] Error in the arnied API (params_count={params_count})")
+        super().__init__(
+            f"[ShowerrVarMissing] Error in the arnied API (params_count={params_count})")
         self.params_count = params_count
 
+
 class ProviderNotFound(Exception):
     def __init__(self, provider_id: int) -> None:
-        super().__init__(f"[ProviderNotFound] Not found a provider with ID `{provider_id})`")
+        super().__init__(
+            f"[ProviderNotFound] Could not find provider (provider_id={provider_id})")
         self.provider_id = provider_id
 
 
 # Arnied varlink proxy
 
 
-class Arnied(object):
+class Arnied:
+    """
+    Expose methods of the arnied varlink interface in python.
+
+    As described in module doc, documentation of exceptions, methods, and
+    their arguments can be found in arnied source code.
+    """
     VARLINK_ADDRESS = f"unix:{ARNIED_SOCKET}"
 
     @classmethod
@@ -238,12 +269,14 @@ class Arnied(object):
             return conn.SetCnf(vars)
 
     @classmethod
-    def set_commit_cnf(cls, vars: typing.List[CnfVar], username: typing.Optional[str], nogenerate: bool, fix_commit: bool) -> SetCommitCnf:
+    def set_commit_cnf(cls, vars: typing.List[CnfVar], username: typing.Optional[str],
+                       nogenerate: bool, fix_commit: bool) -> SetCommitCnf:
         with cls.new_connection() as conn:
             return conn.SetCommitCnf(vars, username, nogenerate, fix_commit)
 
     @classmethod
-    def commit_cnf(cls, change_numbers: typing.List[int], username: typing.Optional[str], nogenerate: bool, fix_commit: bool) -> CommitCnfRet:
+    def commit_cnf(cls, change_numbers: typing.List[int], username: typing.Optional[str],
+                   nogenerate: bool, fix_commit: bool) -> CommitCnfRet:
         with cls.new_connection() as conn:
             return conn.CommitCnf(change_numbers, username, nogenerate, fix_commit)
 
index dd9a20e..3d60a25 100644 (file)
@@ -2,5 +2,5 @@ from .model import Cnf, CnfList
 from .binary import CnfBinary
 from .store import CnfStore, BinaryCnfStore, CommitException
 
-__all__ = ["Cnf, CnfList, CnfBinary", "CnfStore",
+__all__ = ["Cnf", "CnfList", "CnfBinary", "CnfStore",
            "BinaryCnfStore", "CommitException"]
index 3d71c19..63311cc 100644 (file)
 """
 binary: wrappers around binaries that work with the CNF store.
 
-Featuring
-- CnfBinary: stateless class that can be used to invoke the different binaries
-in a Python-friendly way.
+Featuring:
+    - CnfBinary: stateless class that can be used to invoke the different binaries
+      in a Python-friendly way.
 
 .. note:: It is written as a class on purpose for it to be easily extended to
-support invoking non-local binaries, but methods are all class methods since
-the class is stateless.
+          support invoking non-local binaries, but methods are all class methods since
+          the class is stateless.
+
+.. seealso:: Overview Diagram linked to from doc main page
 
 .. codeauthor:: Intra2net
 """
@@ -38,7 +40,7 @@ import shlex
 import html
 import re
 
-log = logging.getLogger("pyi2ncommon.cnfvar.store")
+log = logging.getLogger("pyi2ncommon.cnfvar.binary")
 
 #: default get_cnf binary
 BIN_GET_CNF = "/usr/intranator/bin/get_cnf"
@@ -48,7 +50,7 @@ BIN_SET_CNF = "/usr/intranator/bin/set_cnf"
 ENCODING = "latin1"
 
 
-class CnfBinary(object):
+class CnfBinary:
     """Provide wrappers around the multiple binaries to handle the CNF store."""
 
     @classmethod
@@ -93,7 +95,7 @@ class CnfBinary(object):
         :rtype: str
 
         .. note:: being a wrapper, this function does not do anything extra
-        like checking if arnied is running or waiting for generate.
+                  like checking if arnied is running or waiting for generate.
         """
         if name is None and instance is not None:
             raise ValueError("cannot pass `instance` without a `name`")
@@ -137,7 +139,7 @@ class CnfBinary(object):
         :raises: :py:class:`SetCnfException` in case the binary errors out
 
         .. note:: being a wrapper, this function does not do anything extra
-        like checking if arnied is running or waiting for generate.
+                  like checking if arnied is running or waiting for generate.
         """
         if input_str is None and input_file is None:
             raise ValueError("Need to pass either a string or a file")
index 3c3fbbf..242a9d3 100644 (file)
 """
 model: Cnf classes, collection of Cnf classes and multiple filtering methods.
 
-Featuring
-- Cnf: class representing a CNF variabe
-
-- CnfList: a collection of `Cnf` instances
+Featuring:
+    - Cnf: class representing a CNF variable
+    - CnfList: a collection of `Cnf` instances
 
 The classes above inherit from their base types with added mixins which
 extend them with extra functionality.
 
+.. seealso:: Overview Diagram linked to from doc main page
+
 .. codeauthor:: Intra2net
 """
 
@@ -64,11 +65,11 @@ class CnfName(str):
     def __contains__(self, name):
         return name.lower() in self.lower()
 
-    def startswith(self, prefix):
-        return self.lower().startswith(prefix.lower())
+    def startswith(self, prefix, *args, **kwargs):
+        return self.lower().startswith(prefix.lower(), *args, **kwargs)
 
-    def endswith(self, prefix):
-        return self.lower().endswith(prefix.lower())
+    def endswith(self, prefix, *args, **kwargs):
+        return self.lower().endswith(prefix.lower(), *args, **kwargs)
 
 
 ###############################################################################
@@ -104,6 +105,7 @@ class BaseCnfList(list):
         else:
             iter_ = []
         super().__init__(iter_)
+        self._renumber_counter = None     # initialized and used in renumber
         if renumber:
             self.renumber()
 
@@ -112,11 +114,11 @@ class BaseCnfList(list):
         # NOTE: we don't keep track of operations that change the list as this
         # would require us to reimplement most of the methods. At least for now
         # this method should be called again when serializing.
-        self._counter = 0
+        self._renumber_counter = 0
 
         def renumber_fn(cnf):
-            self._counter += 1
-            cnf.lineno = self._counter
+            self._renumber_counter += 1
+            cnf.lineno = self._renumber_counter
 
         self.for_each_all(renumber_fn)
 
@@ -126,7 +128,7 @@ class BaseCnfList(list):
 
         :param where_filter: predicate to apply against CNFs
         :type where_filter: function accepting a CNF and returning a boolean
-        :returns: a instance of this class with filtered members
+        :returns: an instance of this class with filtered members
         :rtype: :py:class:`CnfList`
         """
         return CnfList(c for c in self if where_filter(c))
@@ -137,7 +139,7 @@ class BaseCnfList(list):
 
         :param where_filter: predicate to apply against children
         :type where_filter: function accepting a CNF and returning a boolean
-        :returns: a instance of this class with filtered members
+        :returns: an instance of this class with filtered members
         :rtype: :py:class:`CnfList`
         """
         def upper_filter(cnf):
@@ -530,7 +532,7 @@ class CnfListSerializationMixin(BaseCnfList):
                 d["children"] = [_to_dict(c) for c in cnf.children]
             return d
         if renumber:
-            renumber = True
+            self.renumber()
         json_list = [_to_dict(c) for c in self]
         return json.dumps({"cnf": json_list})
 
@@ -813,7 +815,7 @@ class CnfShortcutsMixin(BaseCnf):
         """
         cnf = self.children.first_with_name(name, default=None)
         if cnf is None:
-            cnf = self.add_child(name, "1")
+            self.add_child(name, "1")
         else:
             cnf.enable()
 
@@ -827,7 +829,7 @@ class CnfShortcutsMixin(BaseCnf):
         """
         cnf = self.children.first_with_name(name, default=None)
         if cnf is None:
-            cnf = self.add_child(name, "0")
+            self.add_child(name, "0")
         else:
             cnf.disable()
 
index 9872c16..0bfeb26 100644 (file)
 # Copyright (c) 2016-2022 Intra2net AG <info@intra2net.com>
 
 """
-store: implementations of CNF stores using varlink and *et_cnf binaries.
+store: implementations of CNF stores using varlink and `*et_cnf` binaries.
 
-Featuring
-- CnfStore: the main store class that is implemented using the varlink API
+Featuring:
+    - CnfStore: the main store class that is implemented using the varlink API
+    - BinaryCnfStore: alternative store class implementation using the `set_cnf`
+      and `get_cnf` binaries
 
-- BinaryCnfStore: alternative store class implementation using the set_ and
-get_cnf binaries
+.. seealso:: Overview Diagram linked to from doc main page
 
 .. codeauthor:: Intra2net
 """
@@ -89,7 +90,7 @@ class CnfStore:
         :param bool fix_problems: whether to automatically fix errors in the vars
 
         .. note:: you can mix variables to insert and variables to update
-        in the same list as the system should handle it nicely
+                  in the same list as the system should handle it nicely
 
         Example::
             store = CnfStore()
@@ -241,7 +242,7 @@ class CnfStore:
 
 
 class BinaryCnfStore(CnfStore):
-    """Implementation of the CNF store that uses get_ and set_cnf."""
+    """Implementation of the CNF store that uses `get_cnf` and `set_cnf`."""
 
     #: how much to wait for arnied to report running
     ARNIED_TIMEOUT = 30
@@ -272,7 +273,7 @@ class BinaryCnfStore(CnfStore):
         """
         log.debug("Querying BinaryCnfStore with name=%s and instance=%s",
                   name, instance)
-        output = self._driver.get_cnf(name, instance)
+        output = self._driver.get_cnf(name, instance=instance)
 
         if len(output) == 0:
             # otherwise cnfvar raises a generic Malformed exception
@@ -289,7 +290,7 @@ class BinaryCnfStore(CnfStore):
         :param bool fix_problems: whether to automatically fix errors in the vars
 
         .. note:: you can mix variables to insert and variables to update
-        in the same list as the system should handle it nicely
+                  in the same list as the system should handle it nicely
 
         Example::
             store = CnfStore()
@@ -301,7 +302,7 @@ class BinaryCnfStore(CnfStore):
         cnf = self._cnf_or_list(cnf, operation="commit")
         self._autofix_instances(cnf)
 
-        # set_cnf is demaning on lineno's
+        # set_cnf is demanding on lineno's
         cnf.renumber()
         log.debug("Committing variables via binaries:\n%s", cnf)
 
@@ -330,7 +331,7 @@ class BinaryCnfStore(CnfStore):
         if any((c.parent is not None for c in cnf)):
             raise RuntimeError("Calling delete is only supported on top-level CNF variables")
 
-        # set_cnf is demaning on lineno's
+        # set_cnf is demanding on lineno's
         cnf.renumber()
         log.debug("Deleting variables via binaries:\n%s", cnf)
 
@@ -353,6 +354,18 @@ class BinaryCnfStore(CnfStore):
         return fn(*args, **kwargs)
 
 
+#: pattern for more verbose error message for :py:class:`CommitException`
+COMMIT_EXCEPTION_MESSAGE = """\
+Error committing CNF variables!
+----------------------------
+Input:
+{cnfvars}
+----------------------------
+Error:
+{msg}
+"""
+
+
 class CommitException(Exception):
     """Custom exception for commit errors."""
 
@@ -364,13 +377,5 @@ class CommitException(Exception):
         :type cnfvars: CnfList
         :param str msg: error message
         """
-        self.message = f"""\
-Error committing CNF variables!
-----------------------------
-Input:
-{cnfvars}
-----------------------------
-Error:
-{msg}
-"""
         super().__init__(msg)
+        self.message = COMMIT_EXCEPTION_MESSAGE.format(cnfvars=cnfvars, msg=msg)
index cd33cf0..45680a8 100644 (file)
@@ -242,7 +242,7 @@ def print(text, *args, **kwargs):           # pylint: disable=redefined-builtin
     """ like the builtin print function but also accepts color args
 
     If any arg of :py:func:`colored` is given in `kwargs`, will run text with
-    color-args through that function. Runs built-in :py:builtin:`print`
+    color-args through that function. Runs built-in :py:func:`print`
     function with result and other args.
 
     ...todo:: color all args, not just first
index e234c4d..b5612d3 100644 (file)
@@ -57,7 +57,8 @@ class TestStore(unittest.TestCase):
              patch.object(store, "_wait_for_generate"):
             self._query_and_update(store)
 
-        cnf_api_input = set_cnf_mock.call_args.kwargs["vars"]
+        args, kwargs = set_cnf_mock.call_args
+        cnf_api_input = kwargs["vars"]
         cnf_input = CnfList.from_api_structure(cnf_api_input)
 
         expected_input = dedent("""\
@@ -100,7 +101,8 @@ class TestStore(unittest.TestCase):
              patch.object(store, "_wait_for_generate"):
             store.delete(cnfvars)
 
-        cnf_api_input = set_cnf_mock.call_args.kwargs["vars"]
+        args, kwargs = set_cnf_mock.call_args
+        cnf_api_input = kwargs["vars"]
         cnf_input = CnfList.from_api_structure(cnf_api_input)
 
         expected_input = dedent("""\