logging and typing improvements

Author: fredrikt

src/pyff/builtins.py

@@ -512,7 +512,7 @@ def publish(req: Plumbing.Request, *opts):
         try:
             validate_document(req.t)
         except DocumentInvalid as ex:
-            log.error(ex.error_log)
+            log.error(f'Got an error from validate_document {ex.error_log}')
             raise PipeException("XML schema validation failed")
 
     def _nop(x):

src/pyff/repo.py

@@ -1,10 +1,13 @@
 import random
+from typing import Iterable, Optional
+
+from lxml.etree import Element
 
 from pyff.constants import NS, config
 from pyff.logs import get_log
 from pyff.resource import Resource, ResourceOpts
 from pyff.samlmd import entitiesdescriptor, root
-from pyff.store import make_icon_store_instance, make_store_instance
+from pyff.store import SAMLStoreBase, make_icon_store_instance, make_store_instance
 from pyff.utils import is_text, make_default_scheduler
 
 log = get_log(__name__)
@@ -43,17 +46,14 @@ def _lookup(self, member, store=None):
         log.debug("calling store lookup %s" % member)
         return store.lookup(member)
 
-    def lookup(self, member, xp=None, store=None):
+    def lookup(self, member: str, xp: str = None, store: Optional[SAMLStoreBase] = None) -> Iterable[Element]:
         """
         Lookup elements in the working metadata repository
 
         :param member: A selector (cf below)
-        :type member: basestring
         :param xp: An optional xpath filter
-        :type xp: basestring
         :param store: the store to operate on
         :return: An iterable of EntityDescriptor elements
-        :rtype: etree.Element
 
 
         **Selector Syntax**
@@ -67,7 +67,7 @@ def lookup(self, member, xp=None, store=None):
         The first form results in the intersection of the results of doing a lookup on the selectors. The second form
         results in the EntityDescriptor elements from the source (defaults to all EntityDescriptors) that match the
         xpath expression. The attribute-value forms results in the EntityDescriptors that contain the specified entity
-        attribute pair. If non of these forms apply, the lookup is done using either source ID (normally @Name from
+        attribute pair. If none of these forms apply, the lookup is done using either source ID (normally @Name from
         the EntitiesDescriptor) or the entityID of single EntityDescriptors. If member is a URI but isn't part of
         the metadata repository then it is fetched an treated as a list of (one per line) of selectors. If all else
         fails an empty list is returned.

src/pyff/resource.py

@@ -11,7 +11,7 @@
 from datetime import datetime
 from enum import Enum
 from threading import Condition, Lock
-from typing import Any, Callable, Deque, Dict, Iterable, List, Mapping, Optional, TYPE_CHECKING, Tuple
+from typing import TYPE_CHECKING, Any, Callable, Deque, Dict, Iterable, List, Mapping, Optional, Tuple
 from urllib.parse import quote as urlescape
 
 import requests

src/pyff/samlmd.py

@@ -4,7 +4,7 @@
 from distutils.util import strtobool
 from io import BytesIO
 from itertools import chain
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Sequence, Union
 
 from lxml import etree
 from lxml.builder import ElementMaker
@@ -364,7 +364,7 @@ def filter_or_validate(
     return t
 
 
-def resolve_entities(entities, lookup_fn=None):
+def resolve_entities(entities: Sequence[str], lookup_fn: str = None) -> List[Element]:
     """
 
     :param entities: a set of entities specifiers (lookup is used to find entities from this set)
@@ -384,7 +384,7 @@ def _resolve(m, l_fn):
             entity_id = entity.get('entityID', None)
             if entity is not None and entity_id is not None:
                 resolved_entities[entity_id] = entity
-    return resolved_entities.values()
+    return list(resolved_entities.values())
 
 
 def entitiesdescriptor(

src/pyff/test/__init__.py

@@ -96,7 +96,8 @@ def run_cmdline(script, *args):
 
 def _pstart(args, outf=None, ignore_exit=False):
     env = {}
-    logging.debug(" ".join(args))
+    _cmdstr = ' '.join(args)
+    logging.debug(f'Invoking external command via pipe: {_cmdstr}')
     proc = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
     return proc
 
@@ -105,13 +106,14 @@ def _p(args, outf=None, ignore_exit=False):
     proc = _pstart(args)
     out, err = proc.communicate()
     if err is not None and len(err) > 0:
-        logging.error(err)
+        logging.error(f'stderr from command: {err}')
     if outf is not None:
         with open(outf, "w") as fd:
             fd.write(out.decode('UTF-8'))
     else:
         if out is not None and len(out) > 0:
-            logging.debug(out.decode('UTF-8'))
+            _outstr = out.decode('UTF-8')
+            logging.debug(f'stdout from command: {_outstr}')
     rv = proc.wait()
     if rv and not ignore_exit:
         raise RuntimeError("command exited with code != 0: %d" % rv)

src/pyff/test/test_utils.py

@@ -8,6 +8,7 @@
 
 from pyff import utils
 from pyff.constants import NS, as_list_of_string
+from pyff.merge_strategies import remove, replace_existing
 from pyff.resource import Resource, ResourceOpts
 from pyff.samlmd import entities_list, find_entity
 from pyff.utils import (
@@ -24,8 +25,6 @@
     url_get,
 )
 
-from pyff.merge_strategies import remove, replace_existing
-
 
 class TestMetadata(TestCase):
     def setUp(self):

src/pyff/utils.py

@@ -18,7 +18,6 @@
 import threading
 import time
 import traceback
-from _collections_abc import Mapping, MutableMapping
 from copy import copy
 from datetime import datetime, timedelta, timezone
 from email.utils import parsedate
@@ -30,6 +29,7 @@
 import pkg_resources
 import requests
 import xmlsec
+from _collections_abc import Mapping, MutableMapping
 from apscheduler.executors.pool import ThreadPoolExecutor
 from apscheduler.jobstores.memory import MemoryJobStore
 from apscheduler.jobstores.redis import RedisJobStore