fix: userviced_requests fix #110

Author: vetsin

docs/user/advanced.rst

@@ -132,3 +132,7 @@ The following can improve performance:
 * Increase (or decrease) the default :ref:`batch_size` for GlideRecord
 * According to `KB0534905 <https://support.servicenow.com/kb_view.do?sysparm_article=KB0534905>`_ try disabling display values if they are not required via `gr.display_value = False`
 * Try setting a query :ref:`limit` if you do not need all results
+
+2. Why am I consuming so much memory?
+
+By default, GlideRecord can be 'rewound' in that it stores all previously fetched records in memory. This can be disabled by setting :ref:`rewind` to False on GlideRecord

pysnc/client.py

@@ -365,7 +365,14 @@ def _transform_response(self, req: requests.PreparedRequest, serviced_request) -
 
         return response
 
-    def execute(self):
+    def execute(self, attempt=0):
+        if attempt > 2:
+            # just give up and tell em we tried
+            for h in self.__hooks:
+                self.__hooks[h](None)
+            self.__hooks = {}
+            self.__requests = []
+            self.__stored_requests = {}
         bid = self._next_id()
         body = {
             'batch_request_id': bid,
@@ -374,20 +381,18 @@ def execute(self):
         r = self.session.post(self._batch_target(), json=body)
         self._validate_response(r)
         data = r.json()
-        try:
-            assert str(bid) == data['batch_request_id'], f"How did we get a response id different from {bid}"
-
-            for response in data['serviced_requests'] + data['unserviced_requests']:
-                response_id = response['id']
-                assert response_id in self.__hooks, f"Somehow has no hook for {response_id}"
-                assert response_id in self.__stored_requests, f"Somehow we did not store request for {response_id}"
-                self.__hooks[response['id']](self._transform_response(self.__stored_requests[response_id], response))
-
-            return bid
-        finally:
-            self.__stored_requests = {}
-            self.__requests = []
-            self.__hooks = {}
+        assert str(bid) == data['batch_request_id'], f"How did we get a response id different from {bid}"
+
+        for response in data['serviced_requests']:
+            response_id = response['id']
+            assert response_id in self.__hooks, f"Somehow has no hook for {response_id}"
+            assert response_id in self.__stored_requests, f"Somehow we did not store request for {response_id}"
+            self.__hooks[response['id']](self._transform_response(self.__stored_requests.pop(response_id), response))
+            del self.__hooks[response_id]
+            self.__requests = list(filter(lambda x: x['id'] != response_id, self.__requests))
+
+        if len(data['unserviced_requests']) > 0:
+            self.execute(attempt=attempt+1)
 
     def get(self, record: GlideRecord, sys_id: str, hook: Callable) -> None:
         params = self._set_params(record)

pysnc/record.py

@@ -706,7 +706,7 @@ def delete_multiple(self) -> bool:
         allRecordsWereDeleted = True
         def handle(response):
             nonlocal  allRecordsWereDeleted
-            if response.status_code != 204:
+            if response is None or response.status_code != 204:
                 allRecordsWereDeleted = False
 
         for e in self:
@@ -716,12 +716,17 @@ def handle(response):
 
     def update_multiple(self, custom_handler=None) -> bool:
         """
-        Updates multiple records at once
+        Updates multiple records at once. A ``custom_handler`` of the form ``def handle(response: requests.Response | None)`` can be passed in,
+        which may be useful if you wish to handle errors in a specific way. Note that if a custom_handler is used this
+        method will always return ``True``
+
+
+        :return: ``True`` on success, ``False`` if any records failed. If custom_handler is specified, always returns ``True``
         """
         updated = True
         def handle(response):
             nonlocal updated
-            if response.status_code != 200:
+            if response is None or response.status_code != 200:
                 updated = False
 
         for e in self:

test/test_snc_api_write.py

@@ -254,5 +254,74 @@ def custom_handle(response):
         tgr.delete_multiple()
         client.session.close()
 
+    def test_multi_update_with_failures(self):
+        client = ServiceNowClient(self.c.server, self.c.credentials)
+        br = client.GlideRecord('sys_script')
+
+        # in order to test the failure case, let's insert a BR that will reject specific values
+        br.add_query('name', 'test_multi_update_with_failures')
+        br.query()
+        if not br.next():
+            br.initialize()
+            br.name = 'test_multi_update_with_failures'
+            br.collection = 'problem'
+            br.active = True
+            br.when = 'before'
+            br.order = 100
+            br.action_insert = True
+            br.action_update = True
+            br.abort_action = True
+            br.add_message = True
+            br.message = 'rejected by test_multi_update_with_failures br'
+            br.filter_condition = 'short_descriptionLIKEEICAR^ORdescriptionLIKEEICAR^EQ'
+            br.insert()
+
+
+        gr = client.GlideRecord('problem')
+        gr.add_query('short_description', 'LIKE', 'BUNKZZ')
+        gr.query()
+        self.assertTrue(gr.delete_multiple()) # try to make sure weh ave none first
+        gr.query()
+        self.assertEqual(len(gr), 0, 'should have had none left')
+
 
+        total_count = 10
+        # insert five bunk records
+        # TODO: insert multiple
+        for i in range(total_count):
+            gr = client.GlideRecord('problem')
+            gr.initialize()
+            gr.short_description = f"Unit Test - BUNKZZ Multi update {i}"
+            assert gr.insert(), "Failed to insert a record"
+
+        gr = client.GlideRecord('problem')
+        gr.add_query('short_description', 'LIKE', 'BUNKZZ')
+        gr.query()
+        self.assertEqual(len(gr), total_count)
+
+        i = 0
+        # half append
+        print(f"for i < {total_count//2}")
+        while i < (total_count//2) and gr.next():
+            gr.short_description = gr.short_description + ' -- APPENDEDZZ'
+            i += 1
+        # half error
+        while gr.next():
+            gr.short_description = gr.short_description + ' -- EICAR'
+
+        self.assertFalse(gr.update_multiple())
+        # make sure we cleaned up as expected
+        self.assertEqual(gr._client.batch_api._BatchAPI__hooks, {})
+        self.assertEqual(gr._client.batch_api._BatchAPI__stored_requests, {})
+        self.assertEqual(gr._client.batch_api._BatchAPI__requests, [])
+
+        tgr = client.GlideRecord('problem')
+        tgr.add_query('short_description', 'LIKE', 'APPENDEDZZ')
+        tgr.query()
+        self.assertEqual(len(tgr), total_count//2)
+
+        tgr = client.GlideRecord('problem')
+        tgr.add_query('short_description', 'LIKE', 'EICAR')
+        tgr.query()
+        self.assertEqual(len(tgr), 0)