Repository: juniper/py-junos-eznc
Issue: Utils.sw.checksum Timeout

Modify remote_checksum to accept timeout variable (set default to 300) and pass into RPC as dev_timeout (to use timeout dec).

Juniper/ansible-junos-stdlib#64


Changed files:
lib/jnpr/junos/exception.py, lib/jnpr/junos/utils/sw.py, tests/unit/utils/test_config.py, tests/unit/utils/test_sw.py, lib/jnpr/junos/utils/config.py

diff --git a/lib/jnpr/junos/exception.py b/lib/jnpr/junos/exception.py
index 6391efd8..312d878d 100644
--- a/lib/jnpr/junos/exception.py
+++ b/lib/jnpr/junos/exception.py
@@ -1,4 +1,5 @@
 from jnpr.junos import jxml
+from lxml.etree import _Element
 
 
 class RpcError(Exception):
@@ -20,15 +21,18 @@ class RpcError(Exception):
         self.dev = dev
         self.timeout = timeout
         self.re = re
+        self.rpc_error = None
+
+        if isinstance(self.rsp, _Element):
+            self.rpc_error = jxml.rpc_error(self.rsp)
+            if self.errs is None:
+                self.errs = self.rpc_error
 
     def __repr__(self):
         """
           pprints the response XML attribute
         """
-        if self.rsp is not None:
-            self.rpc_error = jxml.rpc_error(self.rsp)
-            if self.errs is None:
-                self.errs = self.rpc_error
+        if self.rpc_error is not None:
             return "{0}(severity: {1}, bad_element: {2}, message: {3})"\\
                 .format(self.__class__.__name__, self.rpc_error['severity'],
                         self.rpc_error['bad_element'], self.rpc_error['message'])
@@ -40,11 +44,8 @@ class CommitError(RpcError):
     """
     Generated in response to a commit-check or a commit action.
     """
-    def __init__(self, cmd=None, rsp=None, errs=None):
+    def __init__(self, rsp, cmd=None, errs=None):
         RpcError.__init__(self, cmd, rsp, errs)
-        self.rpc_error = jxml.rpc_error(rsp)
-        if self.errs is None:
-            self.errs = self.rpc_error
 
     def __repr__(self):
         return "{0}(edit_path: {1}, bad_element: {2}, message: {3})"\\
@@ -58,11 +59,8 @@ class ConfigLoadError(RpcError):
     """
     Generated in response to a failure when loading a configuration.
     """
-    def __init__(self, cmd=None, rsp=None, errs=None):
+    def __init__(self, rsp, cmd=None, errs=None):
         RpcError.__init__(self, cmd, rsp, errs)
-        self.rpc_error = jxml.rpc_error(rsp)
-        if self.errs is None:
-            self.errs = self.rpc_error
 
     def __repr__(self):
         return "{0}(severity: {1}, bad_element: {2}, message: {3})"\\
@@ -79,7 +77,6 @@ class LockError(RpcError):
     """
     def __init__(self, rsp):
         RpcError.__init__(self, rsp=rsp)
-        self.rpc_error = jxml.rpc_error(rsp)
 
 
 class UnlockError(RpcError):
@@ -89,7 +86,6 @@ class UnlockError(RpcError):
     """
     def __init__(self, rsp):
         RpcError.__init__(self, rsp=rsp)
-        self.rpc_error = jxml.rpc_error(rsp)
 
 
 class PermissionError(RpcError):
@@ -100,7 +96,7 @@ class PermissionError(RpcError):
     PermissionError.message gives you the specific RPC that cause
     the exceptions
     """
-    def __init__(self, cmd=None, rsp=None):
+    def __init__(self, rsp, cmd=None):
         RpcError.__init__(self, cmd=cmd, rsp=rsp)
         self.message = rsp.findtext('.//bad-element')
 
@@ -123,7 +119,7 @@ class SwRollbackError(RpcError):
     """
     Generated in response to a SW rollback error.
     """
-    def __init__(self, re=None, rsp=None):
+    def __init__(self, rsp, re=None):
         RpcError.__init__(self, re=re, rsp=rsp)
 
     def __repr__(self):
diff --git a/lib/jnpr/junos/utils/config.py b/lib/jnpr/junos/utils/config.py
index 99abf33a..2946c0bb 100644
--- a/lib/jnpr/junos/utils/config.py
+++ b/lib/jnpr/junos/utils/config.py
@@ -129,8 +129,10 @@ class Config(Util):
         except Exception as err:
             # so the ncclient gives us something I don't want.  I'm going to
             # convert it and re-raise the commit error
-            JXML.remove_namespaces(err.xml)
-            raise CommitError(rsp=err.xml)
+            if hasattr(err, 'xml') and isinstance(err.xml, etree._Element):
+                raise CommitError(rsp=err.xml)
+            else:
+                raise
 
         if detail:
             return rsp
diff --git a/lib/jnpr/junos/utils/sw.py b/lib/jnpr/junos/utils/sw.py
index 24c41671..6cc244fd 100644
--- a/lib/jnpr/junos/utils/sw.py
+++ b/lib/jnpr/junos/utils/sw.py
@@ -10,7 +10,7 @@ from lxml.builder import E
 # local modules
 from jnpr.junos.utils.util import Util
 from jnpr.junos.utils.scp import SCP
-from jnpr.junos.exception import SwRollbackError, RpcTimeoutError
+from jnpr.junos.exception import SwRollbackError, RpcTimeoutError, RpcError
 
 """
 Software Installation Utilities
@@ -217,22 +217,33 @@ class SW(Util):
         errcode = int(rsp.findtext('package-result'))
         return True if 0 == errcode else rsp.findtext('output').strip()
 
-    def remote_checksum(self, remote_package):
+    def remote_checksum(self, remote_package, timeout=300):
         """
         Computes the MD5 checksum on the remote device.
 
         :param str remote_package:
             The file-path on the remote Junos device
 
-        :returns:
-            The MD5 checksum string
+        :param int timeout:
+          The amount of time (seconds) before declaring an RPC timeout.
+          The default RPC timeout is generally around 30 seconds.  So this
+          :timeout: value will be used in the context of the checksum process.
+          Defaults to 5 minutes (5*60=300)
 
-        :raises RpcError: when the **remote_package** is not found.
+        :returns:
+            * The MD5 checksum string
+            * ``False`` when the **remote_package** is not found.
 
-        .. todo:: should trap the error and return ``None`` instead.
+        :raises RpcError: RPC errors other than **remote_package** not found.
         """
-        rsp = self.rpc.get_checksum_information(path=remote_package)
-        return rsp.findtext('.//checksum').strip()
+        try:
+            rsp = self.rpc.get_checksum_information(path=remote_package, dev_timeout=timeout)
+            return rsp.findtext('.//checksum').strip()
+        except RpcError as e:
+            if hasattr(e, 'errs') and ('No such file or directory' in e.errs['message']):
+                return None
+            else:
+                raise
 
     # -------------------------------------------------------------------------
     # safe_copy - copies the package and performs checksum
diff --git a/tests/unit/utils/test_config.py b/tests/unit/utils/test_config.py
index 0be1b4ed..b995d49a 100644
--- a/tests/unit/utils/test_config.py
+++ b/tests/unit/utils/test_config.py
@@ -80,12 +80,19 @@ class TestConfig(unittest.TestCase):
                                 **{'synchronize': True, 'full': True, 'force-synchronize': True})
 
     @patch('jnpr.junos.utils.config.JXML.remove_namespaces')
-    def test_config_commit_exception(self, mock_jxml):
+    def test_config_commit_xml_exception(self, mock_jxml):
         class MyException(Exception):
-            xml = 'test'
+            xml = etree.fromstring('<test/>')
+        self.conf.rpc.commit_configuration = \\
+            MagicMock(side_effect=MyException)
+        self.assertRaises(CommitError, self.conf.commit)
+
+    def test_config_commit_exception(self):
+        class MyException(Exception):
+            pass
         self.conf.rpc.commit_configuration = \\
             MagicMock(side_effect=MyException)
-        self.assertRaises(AttributeError, self.conf.commit)
+        self.assertRaises(MyException, self.conf.commit)
 
     def test_config_commit_exception_RpcError(self):
         ex = RpcError(rsp='ok')
diff --git a/tests/unit/utils/test_sw.py b/tests/unit/utils/test_sw.py
index 39dc9b2f..a8728a5c 100644
--- a/tests/unit/utils/test_sw.py
+++ b/tests/unit/utils/test_sw.py
@@ -1,7 +1,7 @@
 __author__ = "Nitin Kumar, Rick Sherman"
 __credits__ = "Jeremy Schulman"
 
-import unittest
+import unittest2 as unittest
 from nose.plugins.attrib import attr
 
 import os
@@ -145,6 +145,31 @@ class TestSW(unittest.TestCase):
         package = 'package.tgz'
         self.assertTrue(self.sw.validate(package))
 
+    @patch('jnpr.junos.Device.execute')
+    def test_sw_remote_checksum_not_found(self, mock_execute):
+        xml = '''<rpc-error>
+        <error-severity>error</error-severity>
+        <error-message>
+        md5: /var/tmp/123: No such file or directory
+        </error-message>
+        </rpc-error>'''
+        mock_execute.side_effect = RpcError(rsp=etree.fromstring(xml))
+        package = 'test.tgz'
+        self.assertEqual(self.sw.remote_checksum(package), None)
+
+    @patch('jnpr.junos.Device.execute')
+    def test_sw_remote_checksum_not_rpc_error(self, mock_execute):
+        xml = '''<rpc-error>
+        <error-severity>error</error-severity>
+        <error-message>
+        something else!
+        </error-message>
+        </rpc-error>'''
+        mock_execute.side_effect = RpcError(rsp=etree.fromstring(xml))
+        package = 'test.tgz'
+        with self.assertRaises(RpcError):
+            self.sw.remote_checksum(package)
+
     @patch('jnpr.junos.Device.execute')
     def test_sw_safe_copy(self, mock_execute):
         mock_execute.side_effect = self._mock_manager
@@ -395,7 +420,7 @@ class TestSW(unittest.TestCase):
     def _mock_manager(self, *args, **kwargs):
         if kwargs:
             # Little hack for mocked execute
-            if kwargs == {'dev_timeout': 1800}:
+            if 'dev_timeout' in kwargs:
                 return self._read_file(args[0].tag + '.xml')
             if 'path' in kwargs:
                 if kwargs['path'] == '/packages':