onaplogging: Docstrings, refactor, type hinting

Identify and document functionalities. Describe parameters
and their types, exception descriptions and types, extensions,
return types and descriptions. Preserve Python 2.7 and 3.x
compatibility. Add Python 2.7 to Tox testing. Extract code to utility files.
Add properties for readability and maintainability, fix naming conventions.
Deprecate old methods and attributes.

Issue-ID: REQ-420
Signed-off-by: Eli Halych <illia.halych@t-mobile.pl>
Change-Id: I19297e40fad743ec68aa04612ecbb11f61f2abec

4 files changed, 461 insertions, 92 deletions

diff --git a/pylog/onaplogging/marker/marker.py b/pylog/onaplogging/marker/marker.py
index 5414e21..17a3328 100644
--- a/pylog/onaplogging/marker/marker.py
+++ b/pylog/onaplogging/marker/marker.py
@@ -14,10 +14,30 @@
 
 import abc
 
+from typing import Iterable, List, Optional, Union, Iterator
+from deprecated import deprecated
+from warnings import warn
+from logging import LogRecord
+
 MARKER_TAG = "marker"
 
 
 class Marker(object):
+    """Abstract class for defining the marker structure.
+
+    TODO:
+        after deprecated child methods are removed, rename them here.
+    Extends:
+        object
+    Method list:
+        getName
+        addChild
+        addChilds
+        removeChild
+        contains
+    Raises:
+        NotImplementedError
+    """
 
     __metaclass__ = abc.ABCMeta
 
@@ -51,92 +71,235 @@ class Marker(object):
 
 
 class BaseMarker(Marker):
+    """Basic marker class.
+
+    It is a marker with base functionalities that add sub-level markers and
+    check  if another marker  exists as the parent itself  or as its  child.
+
+    Extends:
+        Marker
+    Properties:
+        name            : The name of the marker.
+        children (list) : The list of all children (sub-level) markers.
+    Arguments:
+        name (str)      : The name of the marker.
+    Methods:
+        getName         : Returns the name of the marker.
+        addChild        : Adds a sub-level marker.
+        addChilds       : Adds a list of sub-level markers.
+        removeChild     : Removes a specified sub-level marker.
+        contains        : Checks if a sub-level marker exists.
+    """
+
+    @property
+    def name(self):
+        # type: () -> str
+        """Name of the parent marker."""
+        return self.__name
+
+    @property
+    def children(self):
+        # type: () -> List[Marker]
+        """Child markers of one parent marker."""
+        return self.__childs
+
+    @name.setter
+    def name(self, value):
+        # type: (str) -> None
+        self.__name = value
+
+    @children.setter
+    def children(self, value):
+        # type: (List[Marker]) -> None
+        self.__childs = value
+
+    def __init__(self, name):  # type: (str)
+        """
+        Raises:
+            TypeError   : If the `name` parameter  is  not  a string.
+            ValueError  : If the `name` parameter is an empty string.
+        """
 
-    def __init__(self, name):
         super(BaseMarker, self).__init__()
+
         if not isinstance(name, str):
             raise TypeError("not str type")
+
         if name == "":
             raise ValueError("empty value")
+
+        warn("Attribute `__childs` is replaced by the property `children`."
+            "Use children instead.", DeprecationWarning)
+
         self.__name = name
         self.__childs = []
 
-    def getName(self):
-        return self.__name
+    def add_child(self, marker):
+        # type: (Marker) -> None
+        """Append a marker to child markers.
 
-    def __iter__(self):
-        return iter(self.__childs)
+        Use this method to describe a different level of logs. For example,
+        error  log  would use the  ERROR marker.  However it's  possible to
+        create a,  for instance,  TYPE_ERROR  to mark  type related events.
+        In this case TYPE_ERROR will be a child of parent ERROR.
 
-    def __eq__(self, other):
+        Args:
+            marker      : marker describing a different log level.
+        Raises:
+            TypeError   : if the marker object has different type.
+        """
 
-        if not isinstance(other, Marker):
-            return False
-        if id(self) == id(other):
-            return True
+        if not isinstance(marker, Marker):
+            raise TypeError("Bad marker type.                     \
+                             Can only add markers of type Marker. \
+                             Type %s was passed." % type(marker))
 
-        return self.__name == other.getName()
+        if self == marker:
+            return
 
-    def __hash__(self):
-        return hash(self.__name)
+        if marker not in self.children:
+            self.children.append(marker)
 
-    def contains(self, item=None):
+    def add_children(self, markers):
+        # type: (Iterable[List]) -> None
+        """ Append a list of markers to child markers.
+
+        Args:
+            markers     : An iterable object, containing markers.
+        Raises:
+            Exception   : If  `marker` parameter is not iterable.
+        """
+
+        try:
+            iter(markers)
+        except Exception as e:
+            raise e
+
+        for marker in markers:
+            self.children.append(marker)
+
+    def remove_child(self, marker):
+        # type: (Marker) -> None
+        """Use this method to remove a marker from the children list.
 
-        if isinstance(item, Marker):
-            if item == self:
+        Args:
+            marker   : A child marker object.
+        Raises:
+            TypeError: if the marker object has different type.
+        """
+
+        if not isinstance(marker, Marker):
+            raise TypeError("Bad marker type.                     \
+                             Can only add markers of type Marker. \
+                             Type %s was passed." % type(marker))
+
+        if marker in self.children:
+            self.children.remove(marker)
+
+    def contains(self, item=None):
+        # type: (Optional[Union[Marker, str]]) -> bool
+        """
+        Use it to check if a marker exists as a parent itself or its chidren.
+
+        Args:
+            item    : A child marker object. Defaults to None.
+        Returns:
+            bool    : True if the marker exists.
+        """
+
+        warn("`item` argument will be replaced with `marker`. "
+             "Default value None will be removed.",
+              DeprecationWarning)
+        marker = item
+
+        if isinstance(marker, Marker):
+            if marker == self:
                 return True
             return len(list(filter(
-                lambda x: x == item, self.__childs))) > 0
+                lambda x: x == marker, self.children))) > 0
 
-        elif isinstance(item, str):
-            if item == self.__name:
+        elif isinstance(marker, str):
+            if marker == self.name:
                 return True
 
             return len(list(filter(
-                lambda x: x.__name == item, self.__childs))) > 0
+                lambda x: x.name == marker, self.children))) > 0
 
         return False
 
-    def addChild(self, item):
-        if not isinstance(item, Marker):
-            raise TypeError("can only add  (not %s) marker type"
-                            % type(item))
-        if self == item:
-            return
-        if item not in self.__childs:
-            self.__childs.append(item)
+    def __iter__(self):
+        # type: () -> Iterator[List[Marker]]
+        return iter(self.__childs)
+
+    def __hash__(self):
+        # type (): -> int
+        return hash(self.__name)
 
+    def __eq__(self, other):
+        # type: (Marker) -> bool
+        if not isinstance(other, Marker):
+            return False
+        if id(self) == id(other):
+            return True
+
+        return self.__name == other.getName()
+
+    @deprecated(reason="Will be removed. Call the `name` property instead.")
+    def getName(self):
+        """Class attribute getter."""
+        return self.name
+
+    @deprecated(reason="Will be removed. Call add_children(markers) instead.")
     def addChilds(self, childs):
-        try:
-            iter(childs)
-        except Exception as e:
-            raise e
+        """Add a list of sub-level markers. See add_children(markers)"""
+        self.add_children(childs)
 
-        for item in childs:
-            self.addChild(item)
+    @deprecated(reason="Will be removed. Call add_child(marker) instead.")
+    def addChild(self, item):
+        """Add a sub-level marker. See add_child(marker)"""
+        self.add_child(item)
 
+    @deprecated(reason="Will be removed. Call remove_child(marker) instead.")
     def removeChild(self, item):
-        if not isinstance(item, Marker):
-            raise TypeError("can only add  (not %s) marker type"
-                            % type(item))
-        if item in self.__childs:
-            self.__childs.remove(item)
+        """Remove a sub-level marker. See remove_child(marker)"""
+        self.remove_child(item)
 
 
+@deprecated(reason="Will be removed. "
+    "Call match_marker(record, marker_to_match) instead.")
 def matchMarkerHelp(record, markerToMatch):
-
-    marker = getattr(record, MARKER_TAG, None)
-
-    if marker is None or markerToMatch is None:
+    """See match_marker(record, marker_to_match)."""
+    return match_markers(record, markerToMatch)
+
+
+def match_markers(record, marker_to_match):
+    # type: (LogRecord, Union[Marker, List]) -> bool
+    """
+    Use this method to match a marker (or a list of markers) with a LogRecord
+    record.
+
+    Args:
+        record          : a record that may contain a marker.
+        markerToMatch   : a marker or a list of markers.
+    Raises:
+        Exception       : if match check went wrong.
+    Returns:
+        bool            : whether the check can be done or the marker is found.
+    """
+    record_marker = getattr(record, MARKER_TAG, None)
+
+    if record_marker is None or \
+       marker_to_match is None:
         return False
 
-    if not isinstance(marker, Marker):
+    if not isinstance(record_marker, Marker):
         return False
 
     try:
-        if isinstance(markerToMatch, list):
+        if isinstance(marker_to_match, list):
             return len(list(filter(
-                lambda x: marker.contains(x), markerToMatch))) > 0
+                lambda x: record_marker.contains(x), marker_to_match))) > 0
 
-        return marker.contains(markerToMatch)
+        return record_marker.contains(marker_to_match)
     except Exception as e:
         raise e
diff --git a/pylog/onaplogging/marker/markerFactory.py b/pylog/onaplogging/marker/markerFactory.py
index 0705235..a0e9887 100644
--- a/pylog/onaplogging/marker/markerFactory.py
+++ b/pylog/onaplogging/marker/markerFactory.py
@@ -14,12 +14,32 @@
 
 import abc
 import threading
+
+from deprecated import deprecated
+from warnings import warn
+from typing import Dict, Optional
+
+from .marker import Marker
 from .marker import BaseMarker
 
 lock = threading.RLock()
 
 
 class IMarkerFactory(object):
+    """Abstract marker factory for defining structure.
+
+    TODO:
+        after deprecated child methods are removed, rename them here.
+    Extends:
+        object
+    Method list:
+        getMarker
+        deleteMarker
+        exist
+    Raises:
+        NotImplementedError
+    """
+
     __metaclass__ = abc.ABCMeta
 
     @abc.abstractmethod
@@ -36,39 +56,111 @@ class IMarkerFactory(object):
 
 
 class MarkerFactory(IMarkerFactory):
+    """A factory class maganing every marker.
+
+    It is designed to check the existance, create and remove single markers.
+    This class follows a singleton pattern - only one instance can be created.
+
+    Extends:
+        IMarkerFactory
+    Properties:
+        marker_map      : a map of existing markers.
+    Attributes:
+        _instance       : a marker factory instance.
+    Methods:
+        getMarker       : creates a new marker or returns an available one.
+        deleteMarker    : removes a specific marker.
+        exist           : checks if a specific marker exists.
+    """
 
     _instance = None
     _marker_map = {}
 
-    def __new__(cls, *args, **kwargs):
-
-        if cls._instance is None:
-            cls._instance = super(MarkerFactory, cls).__new__(cls)
+    @property
+    def marker_map(self):
+        # type: () -> Dict
+        if not hasattr(self, '_marker_map'):
+            self._marker_map = {}
+        return self._marker_map
+
+    def get_marker(self, name=None):
+        # type: (Optional[str]) -> Marker
+        """
+        Use it to get any marker by its name. If it doesn't exist - it
+        will  create a new  marker that  will be added to the factory.
+        Blocks the thread while executing.
+
+        Args:
+            name        : A marker name. Defaults to None.
+        Raises:
+            ValueError  : If `name` is None.
+        Returns:
+            Marker      : A found or just newly created marker.
+        """
+
+        if name is None:
+            raise ValueError("Marker name is None. Must have a str value.")
 
-        return cls._instance
+        lock.acquire()
 
-    def getMarker(self, marker_name=None):
-        if marker_name is None:
-            raise ValueError("not empty")
+        marker = self.marker_map.get(name, None)
 
-        lock.acquire()
-        marker = self._marker_map.get(marker_name, None)
         if marker is None:
-            marker = BaseMarker(name=marker_name)
-            self._marker_map[marker_name] = marker
+            marker = BaseMarker(name)
+            self.marker_map[name] = marker
+
         lock.release()
 
         return marker
 
-    def deleteMarker(self, marker_name=None):
+    def delete_marker(self, name=None):
+        # type: (Optional[str]) -> bool
+        """
+        Args:
+            name: A marker name. Defaults to None.
+        Returns:
+            bool: The status of deletion.
+        """
+
         lock.acquire()
-        if self.exist(marker_name):
-            del self._marker_map[marker_name]
+        exists = self.exists(name)
+        if exists:
+            del self.marker_map[name]
             return True
         lock.release()
+
         return False
 
+    def exists(self, name=None):
+        # type: (Optional[str]) -> bool
+        """
+        Checks whether the search for a marker returns None and returns the
+        status of the operation.
+
+        Args:
+            name: marker name. Defaults to None.
+        Returns:
+            bool: status of whether the marker was found.
+        """
+        marker = self.marker_map.get(name, None)
+        return marker is not None
+
+    def __new__(cls, *args, **kwargs):
+        if cls._instance is None:
+            cls._instance = super(MarkerFactory, cls).__new__(cls)
+
+        warn("_marker_map attribute will be replaced by marker_map property.",
+              DeprecationWarning)
+        return cls._instance
+
+    @deprecated(reason="Will be removed. Call exists(name) instead.")
     def exist(self, marker_name=None):
+        return self.exists(marker_name)
 
-        return self._marker_map.get(
-            marker_name, None) is not None
+    @deprecated(reason="Will be removed. Call get_marker(name) instead.")
+    def getMarker(self, marker_name=None):
+        return self.get_marker(marker_name)
+
+    @deprecated(reason="Will be removed. Call delete_marker(name) instead.")
+    def deleteMarker(self, marker_name=None):
+        return self.delete_marker(marker_name)
diff --git a/pylog/onaplogging/marker/markerFilter.py b/pylog/onaplogging/marker/markerFilter.py
index 4f49884..a381d8e 100644
--- a/pylog/onaplogging/marker/markerFilter.py
+++ b/pylog/onaplogging/marker/markerFilter.py
@@ -12,21 +12,57 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import sys
-from logging import Filter
-from .marker import matchMarkerHelp
+from logging import Filter, LogRecord
+from warnings import warn
+from typing import List, Optional, Union
+
+from onaplogging.utils.system import is_above_python_2_7
+
+from .marker import match_markers, Marker
 
 
 class MarkerFilter(Filter):
+    """Marker filtering.
+
+    Extends:
+        logging.Filter
+    Properties:
+        marker_to_match (Marker/list): a marker of list of markers.
+    Methods
+        filter: Filter records by the current filter marker(s).
+    """
+
+    @property
+    def markers_to_match(self):
+        # type: () -> Union[Marker, List[Marker]]
+        return self.markersToMatch  # TODO renamed - deprecated
 
-    def __init__(self, name="", markers=None):
-        if sys.version_info > (2, 7):
+    @markers_to_match.setter
+    def markers_to_match(self, value):
+        # type: ( Union[Marker, List[Marker]] ) -> None
+        self.markersToMatch = value
+
+    def __init__(self,
+                 name="",        # type: str
+                 markers=None):  # type: Optional[Union[Marker, List[Marker]]]
+
+        if is_above_python_2_7():
             super(MarkerFilter, self).__init__(name)
+
         else:
             Filter.__init__(self, name)
 
-        self.markerToMatch = markers
+        warn("markersToMatch attribute will be replaced by a property. "
+              "Use markers_to_match property instead.", DeprecationWarning)
+        self.markers_to_match = markers
 
     def filter(self, record):
-        # compare filter's markers with record's marker
-        return matchMarkerHelp(record, self.markerToMatch)
+        # type: (LogRecord) -> bool
+        """Filter by looking for a marker match.
+
+        Args:
+            record: A record to match with the filter(s).
+        Returns:
+            bool: Whether the record matched with the filter(s)
+        """
+        return match_markers(record, self.markers_to_match)
diff --git a/pylog/onaplogging/marker/markerHandler.py b/pylog/onaplogging/marker/markerHandler.py
index e9ce810..36934a8 100644
--- a/pylog/onaplogging/marker/markerHandler.py
+++ b/pylog/onaplogging/marker/markerHandler.py
@@ -12,40 +12,118 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import sys
+from logging import LogRecord
 from logging.handlers import SMTPHandler
-from .marker import matchMarkerHelp
+from typing import Tuple, List, Optional, Union
+
+from onaplogging.utils.system import is_above_python_2_7, is_above_python_3_2
+
+from .marker import match_markers, Marker
 
 
 class MarkerNotifyHandler(SMTPHandler):
+    """Handler for email notification.
+
+    Wraps logging.handler.SMTPHandler and extends it by sending only such
+    notifications which contain certain markers.
+
+    Extends:
+        SMTPHandler
+    Property:
+        markers: A marker or a list of markers.
+    Args:
+        mailhost: A (host, port) tuple.
+        fromaddr: The sender of the email notification.
+        toaddrs: Email notification recepient(s).
+        subject: Email subject.
+        credentials: A (username, password) tuple.
+        secure: For example (TLS). It is used when the
+                                  credentials are supplied.
+        timout: Default is 5.0 seconds. Python version 3.2+
+        markers: A marker or a list of markers.
+    """
+
+    @property
+    def markers(self):
+        # type: () -> Union[Marker, List[Marker]]
+        return self._markers
+
+    @markers.setter
+    def markers(self, value):
+        # type: ( Union[Marker, List[Marker]] ) - None
+        self._markers = value
+
+    def __init__(self,
+                 mailhost,          # type: Tuple
+                 fromaddr,          # type: str
+                 toaddrs,           # type: Union[List[str], str]
+                 subject,           # type: Tuple
+                 credentials=None,  # type: Tuple
+                 secure=None,       # type: Optional[Tuple]
+                 timeout=5.0,       # type: Optional[float]
+                 markers=None     # type: Optional[Union[Marker, List[Marker]]]
+                 ):
+
+        if is_above_python_3_2():
+            super(MarkerNotifyHandler, self). \
+            __init__(  # noqa: E122
+                mailhost,
+                fromaddr,
+                toaddrs,
+                subject,
+                credentials,
+                secure,
+                timeout)
+
+        elif is_above_python_2_7():
+            super(MarkerNotifyHandler, self). \
+            __init__(  # noqa: E122
+                mailhost,
+                fromaddr,
+                toaddrs,
+                subject,
+                credentials,
+                secure)
 
-    def __init__(self, mailhost, fromaddr, toaddrs, subject,
-                 credentials=None, secure=None, timeout=5.0, markers=None):
-
-        if sys.version_info > (3, 2):
-            super(MarkerNotifyHandler, self).__init__(
-                mailhost, fromaddr, toaddrs, subject,
-                credentials, secure, timeout)
-        elif sys.version_info > (2, 7):
-            super(MarkerNotifyHandler, self).__init__(
-                mailhost, fromaddr, toaddrs, subject,
-                credentials, secure)
         else:
             SMTPHandler.__init__(self,
-                                 mailhost, fromaddr, toaddrs, subject,
-                                 credentials, secure)
+                                 mailhost,
+                                 fromaddr,
+                                 toaddrs,
+                                 subject,
+                                 credentials,
+                                 secure)
 
         self.markers = markers
 
     def handle(self, record):
+        # type: (LogRecord) -> bool
+        """
+        Handle a LogRecord record. Send an email notification.
+        """
+        return self.send_notification(record)
+
+    def send_notification(self, record):
+        # type: (LogRecord) -> bool
+        """Email notification handler.
 
-        if self.markers is None:
+        Matches the record with the specific markers set for email
+        notifications. Sends an email notification if that marker(s) matched.
+
+        Args:
+            record (LogRecord): A record that might contain a marker.
+        Returns:
+            bool: Whether a record was passed for emission (to be sent).
+        """
+
+        if  hasattr(self, "markers") and \
+            self.markers is None:
             return False
 
-        if matchMarkerHelp(record, self.markers):
-            if sys.version_info > (2, 7):
+        if match_markers(record, self.markers):
+
+            if is_above_python_2_7():
                 return super(MarkerNotifyHandler, self).handle(record)
-            else:
-                return SMTPHandler.handle(self, record)
+            return SMTPHandler.handle(self, record)
 
         return False