Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: non-rewindable #111

Merged
merged 4 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 99 additions & 160 deletions poetry.lock

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ classifiers = [

[tool.poetry.dependencies]
python = "^3.8"
requests = "2.31.0"
requests = "^2.31.0"
requests-oauthlib = { version = ">=1.2.0", optional = true}
certifi = "2023.7.22"
pip = ">=23.3.1"
certifi = "^2024.7.4"
urllib3 = "^2.0.7"

[tool.poetry.extras]
Expand Down
8 changes: 6 additions & 2 deletions pysnc/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,19 @@ def __init__(self, instance, auth, proxy=None, verify=None, cert=None, auto_retr
self.attachment_api = AttachmentAPI(self)
self.batch_api = BatchAPI(self)

def GlideRecord(self, table, batch_size=100) -> GlideRecord:
def GlideRecord(self, table, batch_size=100, rewindable=True) -> GlideRecord:
"""
Create a :class:`pysnc.GlideRecord` for a given table against the current client

:param str table: The table name e.g. ``problem``
:param int batch_size: Batch size (items returned per HTTP request). Default is ``100``.
:param bool rewindable: If we can rewind the record. Default is ``True``. If ``False`` then we cannot rewind
the record, which means as an Iterable this object will be 'spent' after iteration.
This is normally the default behavior expected for a python Iterable, but not a GlideRecord.
When ``False`` less memory will be consumed, as each previous record will be collected.
:return: :class:`pysnc.GlideRecord`
"""
return GlideRecord(self, table, batch_size)
return GlideRecord(self, table, batch_size, rewindable)

def Attachment(self, table) -> Attachment:
"""
Expand Down
34 changes: 27 additions & 7 deletions pysnc/record.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,12 @@ class GlideRecord(object):
:param ServiceNowClient client: We need to know which instance we're connecting to
:param str table: The table are we going to access
:param int batch_size: Batch size (items returned per HTTP request). Default is ``500``.
:param bool rewindable: If we can rewind the record. Default is ``True``. If ``False`` then we cannot rewind
the record, which means as an Iterable this object will be 'spent' after iteration.
This is normally the default behavior expected for a python Iterable, but not a GlideRecord.
When ``False`` less memory will be consumed, as each previous record will be collected.
"""
def __init__(self, client: 'ServiceNowClient', table: str, batch_size=500):
def __init__(self, client: 'ServiceNowClient', table: str, batch_size: int=500, rewindable: bool=True):
self._log = logging.getLogger(__name__)
self._client = client
self.__table: str = table
Expand All @@ -262,6 +266,7 @@ def __init__(self, client: 'ServiceNowClient', table: str, batch_size=500):
self.__order: str = "ORDERBYsys_id" # we *need* a default order in the event we page, see issue#96
self.__is_new_record: bool = False
self.__display_value: Union[bool, str] = 'all'
self.__rewindable = rewindable

def _clear_query(self):
self.__query = Query(self.__table)
Expand Down Expand Up @@ -443,6 +448,7 @@ def order_by_desc(self, column: str):
def pop_record(self) -> 'GlideRecord':
"""
Pop the current record into a new :class:`GlideRecord` object - equivalent to a clone of a singular record
FIXME: this, by the name, should be a destructive operation, but it is not.

:return: Give us a new :class:`GlideRecord` containing only the current record
"""
Expand Down Expand Up @@ -481,8 +487,10 @@ def set_new_guid_value(self, value):

def rewind(self):
"""
Rewinds the record so it may be iterated upon again. Not required to be called if iterating in the pythonic method.
Rewinds the record (iterable) so it may be iterated upon again. Only possible when the record is rewindable.
"""
if not self._is_rewindable():
raise Exception('Cannot rewind a non-rewindable record')
self.__current = -1

def changes(self) -> bool:
Expand All @@ -506,6 +514,12 @@ def query(self, query=None):
:AuthenticationException: If we do not have rights
:RequestException: If the transaction is canceled due to execution time
"""
if not self._is_rewindable() and self.__current > 0:
raise RuntimeError(f"huh {self._is_rewindable} and {self.__current}")
# raise RuntimeError('Cannot re-query a non-rewindable record that has been iterated upon')
self._do_query(query)

def _do_query(self, query=None):
stored = self.__query
if query:
assert isinstance(query, Query), 'cannot query with a non query object'
Expand Down Expand Up @@ -564,7 +578,7 @@ def get(self, name, value=None) -> bool:
return False
else:
self.add_query(name, value)
self.query()
self._do_query()
return self.next()

def insert(self) -> Optional[GlideElement]:
Expand Down Expand Up @@ -645,7 +659,7 @@ def delete_multiple(self) -> bool:
if self.__total is None:
if not self.__field_limits:
self.fields = 'sys_id' # type: ignore ## all we need...
self.query()
self._do_query()

allRecordsWereDeleted = True
def handle(response):
Expand Down Expand Up @@ -1074,9 +1088,13 @@ def to_pandas(self, columns=None, mode='smart'):

return data

def _is_rewindable(self) -> bool:
return self.__rewindable

def __iter__(self):
self.__is_iter = True
self.rewind()
if self._is_rewindable():
self.rewind()
return self

def __next__(self):
Expand All @@ -1092,6 +1110,8 @@ def next(self, _recursive=False) -> bool:
if l > 0 and self.__current+1 < l:
self.__current = self.__current + 1
if self.__is_iter:
if not self._is_rewindable(): # if we're not rewindable, remove the previous record
self.__results[self.__current - 1] = None
return self # type: ignore # this typing is internal only
return True
if self.__total and self.__total > 0 and \
Expand All @@ -1100,10 +1120,10 @@ def next(self, _recursive=False) -> bool:
_recursive is False:
if self.__limit:
if self.__current+1 < self.__limit:
self.query()
self._do_query()
return self.next(_recursive=True)
else:
self.query()
self._do_query()
return self.next(_recursive=True)
if self.__is_iter:
self.__is_iter = False
Expand Down
3 changes: 2 additions & 1 deletion test/test_snc_auth.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest import TestCase
from unittest import TestCase, skip

from pysnc import ServiceNowClient
from pysnc.auth import *
Expand Down Expand Up @@ -29,6 +29,7 @@ def test_basic_fail(self):
except Exception:
assert 'Should have got an Auth exception'

@skip("Requires valid oauth client_id and secret, and I don't want to need anything not out of box")
def test_oauth(self):
# Manual setup using legacy oauth
server = self.c.server
Expand Down
57 changes: 57 additions & 0 deletions test/test_snc_iteration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
from unittest import TestCase

from pysnc import ServiceNowClient
from constants import Constants
from pprint import pprint

class TestIteration(TestCase):
c = Constants()

def test_default_behavior(self):
client = ServiceNowClient(self.c.server, self.c.credentials)
gr = client.GlideRecord('sys_metadata', batch_size=100)
gr.fields = 'sys_id'
gr.limit = 500
gr.query()
self.assertTrue(gr._is_rewindable())

self.assertTrue(len(gr) > 500, 'Expected more than 500 records')

count = 0
while gr.next():
count += 1
self.assertEqual(count, 500, 'Expected 500 records when using next')

self.assertEqual(len([r.sys_id for r in gr]), 500, 'Expected 500 records when an iterable')
self.assertEqual(len([r.sys_id for r in gr]), 500, 'Expected 500 records when iterated again')

# expect the same for next
count = 0
while gr.next():
count += 1
self.assertEqual(count, 0, 'Expected 0 records when not rewound, as next does not auto-rewind')
gr.rewind()
while gr.next():
count += 1
self.assertEqual(count, 500, 'Expected 500 post rewind')

# should not throw
gr.query()
gr.query()

client.session.close()

def test_rewind_behavior(self):
client = ServiceNowClient(self.c.server, self.c.credentials)
gr = client.GlideRecord('sys_metadata', batch_size=250, rewindable=False)
gr.fields = 'sys_id'
gr.limit = 500
gr.query()
self.assertEqual(gr._GlideRecord__current, -1)
self.assertFalse(gr._is_rewindable())
self.assertEqual(len([r for r in gr]), 500, 'Expected 500 records when an iterable')
self.assertEqual(len([r for r in gr]), 0, 'Expected no records when iterated again')

# but if we query again...
with self.assertRaises(RuntimeError):
gr.query()
Loading