ServiceNow / PySNC

Python API for ServiceNow
MIT License
91 stars 25 forks source link

Update_multiple fails when there are unserviced_requests #110

Open partybison917584020184 opened 2 months ago

partybison917584020184 commented 2 months ago

When using the update_multiple command, I ran into an issue where my instance's batch size is small and had KeyError's from the .execute() iteration over serviced_requests + unserviced_requests coming back in the response. This is because unserviced_requests is returned as a list of the failed request IDs while serviced_requests is a list of completed items as the request dictionary plus execution detail keys.

This can be hotfixed if only we only iterate on serviced_requests, but we can also just keep looping over the unserviced_requests until completed if that is the intention of the batch API. https://github.com/ServiceNow/PySNC/blob/main/pysnc/client.py#L363

    def execute(self):
        bid = self._next_id()
        body = {
            'batch_request_id': bid,
            'rest_requests': self.__requests
        }
        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 = {}

happy to submit patches for either implementation if I could get some guidance :)

Thanks!

vetsin commented 2 months ago

Hard case to test for -- it does sound like they should just be iterated separately.