From ed88a7e69bc53c669f3045c154dc239917dae77c Mon Sep 17 00:00:00 2001
From: Robert Habermann <mail@rhab.de>
Date: Wed, 27 Apr 2016 20:17:36 +0200
Subject: [PATCH] reduce custom Exceptions
---
pyotrs/lib.py | 100 ++++++++++-----------------------
tests/test_client.py | 38 +++++--------
tests/test_pyotrs_responses.py | 4 +-
tests/test_ticket.py | 17 +++---
4 files changed, 56 insertions(+), 103 deletions(-)
diff --git a/pyotrs/lib.py b/pyotrs/lib.py
index d343947..1f75c2a 100644
--- a/pyotrs/lib.py
+++ b/pyotrs/lib.py
@@ -28,15 +28,11 @@ class PyOTRSError(Exception):
self.message = message
-class NoBaseURL(PyOTRSError):
+class ArgumentMissingError(Exception):
pass
-class NoWebServiceName(PyOTRSError):
- pass
-
-
-class NoCredentials(PyOTRSError):
+class ArgumentInvalidError(Exception):
pass
@@ -44,42 +40,10 @@ class ResponseJSONParseError(PyOTRSError):
pass
-class SessionIDStoreNoFileError(PyOTRSError):
- pass
-
-
-class SessionIDStoreNoTimeoutError(PyOTRSError):
- pass
-
-
-class SessionError(PyOTRSError):
- pass
-
-
class SessionCreateError(PyOTRSError):
pass
-class SessionIDFileError(PyOTRSError):
- pass
-
-
-class TicketDynamicFieldsNotRequestedError(PyOTRSError):
- pass
-
-
-class TicketDynamicFieldsParseError(PyOTRSError):
- pass
-
-
-class TicketSearchNothingToLookFor(PyOTRSError):
- pass
-
-
-class TicketSearchNumberMultipleResults(PyOTRSError):
- pass
-
-
class TicketError(PyOTRSError):
pass
@@ -92,10 +56,6 @@ class OTRSHTTPError(PyOTRSError):
pass
-class UpdateAddArticleError(PyOTRSError):
- pass
-
-
class Article(object):
"""PyOTRS Article class """
@@ -393,19 +353,19 @@ class Ticket(object):
"""
if not Title:
- raise ValueError("Title is required")
+ raise ArgumentMissingError("Title is required")
if not Queue and not QueueID:
- raise ValueError("Either Queue or QueueID required")
+ raise ArgumentMissingError("Either Queue or QueueID required")
if not State and not StateID:
- raise ValueError("Either State or StateID required")
+ raise ArgumentMissingError("Either State or StateID required")
if not Priority and not PriorityID:
- raise ValueError("Either Priority or PriorityID required")
+ raise ArgumentMissingError("Either Priority or PriorityID required")
if not CustomerUser:
- raise ValueError("CustomerUser is required")
+ raise ArgumentMissingError("CustomerUser is required")
dct = {u"Title": Title}
@@ -474,8 +434,7 @@ class SessionIDStore(object):
expires (int): seconds as epoch when a session_id record expires
Raises:
- SessionIDStoreNoFileError
- SessionIDStoreNoTimeoutError
+ ArgumentMissingError
.. todo::
Is session_timeout correct here?! Or should this just return "expires" value and caller
@@ -484,10 +443,10 @@ class SessionIDStore(object):
"""
def __init__(self, file_path=None, session_timeout=None):
if not file_path:
- raise SessionIDStoreNoFileError("Argument file_path is required!")
+ raise ArgumentMissingError("Argument file_path is required!")
if not session_timeout:
- raise SessionIDStoreNoTimeoutError("Argument session_timeout is required!")
+ raise ArgumentMissingError("Argument session_timeout is required!")
self.file_path = file_path
self.timeout = session_timeout
@@ -619,8 +578,8 @@ class Client(object):
"""
def __init__(self,
- baseurl,
- webservicename,
+ baseurl=None,
+ webservicename=None,
username=None,
password=None,
session_timeout=None,
@@ -631,11 +590,11 @@ class Client(object):
ca_cert_bundle=None):
if not baseurl:
- raise NoBaseURL("Missing Baseurl (e.g. https://otrs.example.com)")
+ raise ArgumentMissingError("baseurl")
self.baseurl = baseurl.rstrip("/")
if not webservicename:
- raise NoWebServiceName("Missing WebServiceName (e.g. GenericTicketConnectorREST)")
+ raise ArgumentMissingError("webservicename")
self.webservicename = webservicename
if not session_timeout:
@@ -692,7 +651,7 @@ class Client(object):
session_id (unicode): optional If set overrides the self.session_id
Raises:
- SessionError if neither self.session_id nor session_id is not set
+ ArgumentMissingError: if session_id is not set
Returns:
bool: **True** if valid, otherwise **False**
@@ -770,7 +729,7 @@ class Client(object):
# safe new created session_id to file
if not self.session_id_store.write(self.result_json['SessionID']):
- raise SessionIDFileError("Failed to save Session ID to file!")
+ raise IOError("Failed to save Session ID to file!")
else:
print("Saved new Session ID to file: {0}".format(self.session_id_store.file_path))
return True
@@ -804,10 +763,10 @@ class Client(object):
payload = {"SessionID": self.session_id_store.value}
if not ticket:
- raise TicketError("provide Ticket!")
+ raise ArgumentMissingError("Ticket")
if not article:
- raise TicketError("provide Article!")
+ raise ArgumentMissingError("Article")
payload.update(ticket.to_dct())
@@ -912,7 +871,7 @@ class Client(object):
"""
if isinstance(ticket_id_list, int):
- raise ValueError("Please provide list of IDs!")
+ raise ArgumentInvalidError("Please provide list of IDs!")
url = ("{0.baseurl}/otrs/nph-genericinterface.pl/Webservice/"
"{0.webservicename}/TicketList".format(self))
@@ -948,12 +907,16 @@ class Client(object):
dynamic_fields (bool): will request OTRS to include all
Dynamic Fields (*default: True*)
+ Raises:
+ ValueError
+
Returns:
Ticket or False: Ticket object if successful, otherwise **False**
"""
if isinstance(ticket_number, int):
- raise TicketError("Provide ticket_number as str/unicode. Got ticket_number as int.")
+ raise ArgumentInvalidError("Provide ticket_number as str/unicode. "
+ "Got ticket_number as int.")
result_list = self.ticket_search(TicketNumber=ticket_number)
@@ -970,8 +933,8 @@ class Client(object):
return self.result
else:
# TODO
- raise TicketSearchNumberMultipleResults("Found more that one result for "
- "Ticket Number: {0}".format(ticket_number))
+ raise ValueError("Found more that one result for "
+ "Ticket Number: {0}".format(ticket_number))
def _ticket_get_json(self, url, payload):
"""_ticket_get_json
@@ -1035,9 +998,6 @@ class Client(object):
value = value.strftime("%Y-%m-%d %H:%M:%S")
payload.update({key: value})
- if len(payload) == 1:
- raise TicketSearchNothingToLookFor("Nothing specified to look for!")
-
# return self._ticket_search_json(url, payload)
_result = self._ticket_search_json(url, payload)
if not _result:
@@ -1180,7 +1140,7 @@ class Client(object):
payload (dict):
Raises:
- OTRSAPIError
+ ValueError
Returns:
bool: **True** if successful, otherwise **False**
@@ -1200,10 +1160,10 @@ class Client(object):
try:
article_id = self.result_json.get('ArticleID', None)
if not article_id:
- raise UpdateAddArticleError("No new Article was created?!")
+ raise ValueError("No new Article was created?!")
except Exception as err:
- raise UpdateAddArticleError("Unknown Exception: {0}".format(err))
+ raise ValueError("Unknown Exception: {0}".format(err))
return True
@staticmethod
@@ -1263,6 +1223,8 @@ class Client(object):
Raises:
OTRSAPIError
+ NotImplementedError
+ ResponseJSONParseError
Returns:
bool: **True** if successful
diff --git a/tests/test_client.py b/tests/test_client.py
index 2767f49..f954670 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -17,16 +17,13 @@ except ImportError:
import datetime
import requests # noqa
-from pyotrs import Client
from pyotrs import Article, Attachment, Client, DynamicField, Ticket # noqa
-from pyotrs.lib import NoBaseURL, NoWebServiceName, ResponseJSONParseError
-from pyotrs.lib import SessionError
+from pyotrs.lib import ResponseJSONParseError
-from pyotrs.lib import NoBaseURL, NoWebServiceName, NoCredentials # noqa
-from pyotrs.lib import SessionCreateError, SessionIDFileError, OTRSAPIError, OTRSHTTPError # noqa
-from pyotrs.lib import TicketError, TicketSearchNumberMultipleResults # noqa
-from pyotrs.lib import TicketSearchNothingToLookFor # noqa
+from pyotrs.lib import ArgumentMissingError, ArgumentInvalidError # noqa
+from pyotrs.lib import SessionCreateError # noqa
+from pyotrs.lib import OTRSAPIError, OTRSHTTPError # noqa
class ClientTests(unittest.TestCase):
@@ -35,7 +32,7 @@ class ClientTests(unittest.TestCase):
self.assertIsInstance(client, Client)
def test_init_no_base(self):
- self.assertRaisesRegex(NoBaseURL, 'Missing Baseurl.*', Client, '', '')
+ self.assertRaisesRegex(ArgumentMissingError, 'baseurl', Client, '', '')
def test_init_strip_trailing_slash1(self):
obj = Client(baseurl="http://fqdn", webservicename="GenericTicketConnectorREST")
@@ -46,7 +43,7 @@ class ClientTests(unittest.TestCase):
self.assertEqual(obj.baseurl, "http://fqdn")
def test_init_no_webservice(self):
- self.assertRaisesRegex(NoWebServiceName, 'Missing WebSe.*', Client, 'http://localhost', '')
+ self.assertRaisesRegex(ArgumentMissingError, 'webservicename', Client, 'http://localhost')
def test_init_session_id_store(self):
client = Client(baseurl="http://localhost/", webservicename="foo", session_id_file=".sid")
@@ -239,7 +236,7 @@ class ClientTests(unittest.TestCase):
mock_s_create.return_value = True
mock_wr.return_value = False
- self.assertRaisesRegex(SessionIDFileError,
+ self.assertRaisesRegex(IOError,
'Failed to save Session ID to file!',
obj.session_restore_or_set_up_new)
@@ -315,7 +312,7 @@ class ClientTests(unittest.TestCase):
"""Tests ticket_get_by_number provided int not str -> fail"""
obj = Client(baseurl="http://fqdn", webservicename="GenericTicketConnectorREST")
- self.assertRaisesRegex(TicketError,
+ self.assertRaisesRegex(ArgumentInvalidError,
'Provide ticket_number as str/unicode. Got ticket_number as int.',
obj.ticket_get_by_number, ticket_number=1)
@@ -351,7 +348,7 @@ class ClientTests(unittest.TestCase):
obj = Client(baseurl="http://fqdn", webservicename="GenericTicketConnectorREST")
mock_ticket_search.return_value = [1, 2, 3]
- self.assertRaisesRegex(TicketSearchNumberMultipleResults,
+ self.assertRaisesRegex(ValueError,
'Found more that one result for Ticket Number: SomeOtherNumber123',
obj.ticket_get_by_number,
'SomeOtherNumber123')
@@ -363,8 +360,8 @@ class ClientTests(unittest.TestCase):
obj = Client(baseurl="http://fqdn",
webservicename="GenericTicketConnectorREST")
obj.session_id_store.value = "some_session_id"
- self.assertRaisesRegex(TicketError,
- 'provide Ticket!',
+ self.assertRaisesRegex(ArgumentMissingError,
+ 'Ticket',
obj.ticket_create)
def test_ticket_create_no_article(self):
@@ -374,8 +371,8 @@ class ClientTests(unittest.TestCase):
obj.session_id_store.value = "some_session_id"
tic = Ticket(dct={'Title': 'foo'})
- self.assertRaisesRegex(TicketError,
- 'provide Article!',
+ self.assertRaisesRegex(ArgumentMissingError,
+ 'Article',
obj.ticket_create, ticket=tic)
@mock.patch('pyotrs.lib.Client._ticket_create_json', autospec=True)
@@ -446,13 +443,6 @@ class ClientTests(unittest.TestCase):
obj.ticket_create(ticket=tic, article=art, dynamic_field_list=[dyn1, dyn2])
self.assertEqual(mock_t_create_json.call_count, 1)
- def test_ticket_search_nothing_to_look_for(self):
- """Tests ticket_search nothing to look for"""
- obj = Client(baseurl="http://fqdn", webservicename="GenericTicketConnectorREST")
- self.assertRaisesRegex(TicketSearchNothingToLookFor,
- 'Nothing specified to look for!',
- obj.ticket_search)
-
@mock.patch('pyotrs.Client._ticket_search_json', autospec=True)
def test_ticket_search_ticket_id(self, mock_ticket_search_json):
"""Tests ticket_search ticket id"""
@@ -549,7 +539,7 @@ class ClientTests(unittest.TestCase):
att2 = Attachment.create_basic("YmFyCg==", "text/plain", "dümmy.txt")
att2 = Attachment.create_basic("YmFyCg==", "text/plain", "dümmy.txt")
- self.assertRaisesRegex(TicketError,
+ self.assertRaisesRegex(ArgumentMissingError,
'To create an attachment an article is needed!',
obj.ticket_update,
1, attachment_list=[att1, att2])
diff --git a/tests/test_pyotrs_responses.py b/tests/test_pyotrs_responses.py
index 067c11a..6521c62 100644
--- a/tests/test_pyotrs_responses.py
+++ b/tests/test_pyotrs_responses.py
@@ -11,8 +11,8 @@ import responses
import mock
from pyotrs import Client, Ticket # noqa
-from pyotrs.lib import NoBaseURL, NoWebServiceName, NoCredentials # noqa
-from pyotrs.lib import SessionCreateError, SessionIDFileError, OTRSAPIError, OTRSHTTPError # noqa
+from pyotrs.lib import ArgumentMissingError # noqa
+from pyotrs.lib import OTRSAPIError, OTRSHTTPError # noqa
from pyotrs.lib import UpdateAddArticleError # noqa
diff --git a/tests/test_ticket.py b/tests/test_ticket.py
index 48c2613..a9d9d33 100644
--- a/tests/test_ticket.py
+++ b/tests/test_ticket.py
@@ -16,6 +16,7 @@ current_path = os.path.dirname(os.path.realpath(__file__))
sys.path.append(os.path.join(current_path, os.pardir))
from pyotrs import Ticket # noqa
+from pyotrs.lib import ArgumentMissingError
class TicketTests(unittest.TestCase):
@@ -87,30 +88,30 @@ class TicketTests(unittest.TestCase):
self.assertEqual(tic.list_articles[0].__repr__(), "<ArticleID: 30 (1 Attachment)>")
def test_create_basic_no_title(self):
- self.assertRaisesRegex(ValueError, 'Title is required', Ticket.create_basic)
+ self.assertRaisesRegex(ArgumentMissingError, 'Title is required', Ticket.create_basic)
def test_create_basic_title_no_queue(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'Either Queue or QueueID required',
Ticket.create_basic,
Title='foobar')
def test_create_basic_title_queue_no_state1(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'Either State or StateID required',
Ticket.create_basic,
Title='foobar',
Queue='some_queue')
def test_create_basic_title_queue_no_state2(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'Either State or StateID required',
Ticket.create_basic,
Title='foobar',
QueueID='1')
def test_create_basic_title_queue_state_no_prio1(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'Either Priority or PriorityID required',
Ticket.create_basic,
Title='foobar',
@@ -118,7 +119,7 @@ class TicketTests(unittest.TestCase):
State='open')
def test_create_basic_title_queue_state_no_prio2(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'Either Priority or PriorityID required',
Ticket.create_basic,
Title='foobar',
@@ -126,7 +127,7 @@ class TicketTests(unittest.TestCase):
StateID='2')
def test_create_basic_title_queue_state_prio_no_customer1(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'CustomerUser is required',
Ticket.create_basic,
Title='foobar',
@@ -135,7 +136,7 @@ class TicketTests(unittest.TestCase):
Priority='average')
def test_create_basic_title_queue_state_prio_no_customer2(self):
- self.assertRaisesRegex(ValueError,
+ self.assertRaisesRegex(ArgumentMissingError,
'CustomerUser is required',
Ticket.create_basic,
Title='foobar',
--
GitLab