Bugfix add model content (#101)

* rename

* bugfix

* add test for replacing model content

Author: jlwalke2

.github/workflows/build-test-deploy.yml

@@ -1,4 +1,4 @@
-name: "Test & Build"
+name: "Build"
 on: [push, workflow_dispatch]
 jobs:
   codeanalysis:

CHANGELOG.md

@@ -1,10 +1,14 @@
 Unreleased
 ----------
- - 
+**Improvements**
+ - `model_repository.add_model_content()` will now overwrite existing files instead of failing.
+ 
+**Bugfixes**
+ - `PagedList.__repr__()` no longer appears to be an empty list. 
 
 v1.6.0 (2021-06-29)
 -------------------
- **Improvements**
+**Improvements**
  - `Session` now supports authorization using OAuth2 tokens.  Use the `token=` parameter in the constructor when 
  an existing access token token is known.  Alternatively, omitting the `username=` and `password=` parameters
  will now prompt the user for an auth code.

src/sasctl/_services/model_repository.py

@@ -406,7 +406,23 @@ def add_model_content(cls, model, file, name, role=None, content_type=None):
 
         metadata = {'role': role, 'name': name}
 
-        return cls.post('/models/{}/contents'.format(id_), files=files, data=metadata)
+        # return cls.post('/models/{}/contents'.format(id_), files=files, data=metadata)
+
+        # if the file already exists, a 409 error will be returned
+        try:
+            return cls.post('/models/{}/contents'.format(id_),
+                            files=files, data=metadata)
+        # delete the older duplicate model and rerun the API call
+        except HTTPError as e:
+            if e.code == 409:
+                model_contents = cls.get_model_contents(id_)
+                for item in model_contents:
+                    if item.name == name:
+                        cls.delete('/models/{}/contents/{}'.format(id_, item.id))
+                        return cls.post('/models/{}/contents'.format(id_),
+                                        files=files, data=metadata)
+            else:
+                raise e
 
     @classmethod
     def default_repository(cls):

src/sasctl/core.py

@@ -1268,8 +1268,6 @@ def __iter__(self):
         # All Iterators are also Iterables
         return self
 
-    next = __next__  # Python 2 compatible
-
     def _request_async(self, start):
         """Used by worker threads to retrieve next batch of items."""
 
@@ -1338,26 +1336,16 @@ def __next__(self):
         if not self._cache:
             self._cache = next(self._pager)
 
-            if len(self._cache) < self._pager._limit:
-                # number of items returned in page was less than expected
-                # might be last page, or items might have been filtered out by server.
-                pass
-
         # Return the next item
         if self._cache:
             self._count -= 1
             return self._cache.pop(0)
 
         raise StopIteration()
-        # Out of items and out of pages
-        # self._count = 0
-        # raise StopIteration
 
     def __iter__(self):
         return self
 
-    next = __next__  # Python 2 compatible
-
 
 class PagedListIterator:
     """Iterates over an instance of PagedList
@@ -1388,8 +1376,6 @@ def __next__(self):
     def __iter__(self):
         return self
 
-    next = __next__  # Python 2 compatibility
-
 
 class PagedList(list):
     """List that dynamically loads items from the server.
@@ -1418,19 +1404,20 @@ class PagedList(list):
 
     def __init__(self, obj, session=None, threads=4):
         super(PagedList, self).__init__()
-        self._pager = PagedItemIterator(obj, session=session, threads=threads)
+        self._paged_items = PagedItemIterator(obj, session=session, threads=threads)
 
-        # Add the first page of items to the list
-        for _ in range(len(self._pager._cache)):
-            self.append(next(self._pager))
+        # Go ahead and add the items that were initially returned.
+        # Do this by "paging" so iterator remains at the correct spot.
+        for _ in range(len(obj['items'])):
+            self.append(next(self._paged_items))
 
         # Assume that server has more items available
         self._has_more = True
 
     def __len__(self):
         if self._has_more:
             # Estimate the total length as items downloaded + items still on server
-            return super(PagedList, self).__len__() + len(self._pager)
+            return super(PagedList, self).__len__() + len(self._paged_items)
         else:
             # We've pulled everything from the server, so we have an exact length now.
             return super(PagedList, self).__len__()
@@ -1459,7 +1446,7 @@ def __getitem__(self, item):
             # Iterate through server-side pages until we've loaded
             # the item at the requested index.
             while super(PagedList, self).__len__() <= idx:
-                n = next(self._pager)
+                n = next(self._paged_items)
                 self.append(n)
 
         except StopIteration:
@@ -1469,13 +1456,13 @@ def __getitem__(self, item):
         # Get the item from the list
         return super(PagedList, self).__getitem__(item)
 
-    def __str__(self):
-        string = super(PagedList, self).__str__()
+    def __repr__(self):
+        string = super(PagedList, self).__repr__()
 
         # If the list has more "items" than are stored in the underlying list
         # then there are more downloads to make.
         if len(self) - super(PagedList, self).__len__() > 0:
-            string = string.rstrip(']') + ', ... ]'
+            string = string.rstrip(']') + ', ...]'
 
         return string
 

tests/cassettes/test_project_with_sas_and_sklearn_regression_models.test.json

@@ -64,6 +64,10 @@ def test(cas_session, boston_dataset):
     sas_model = register_model(astore, SAS_MODEL_NAME, PROJECT_NAME, force=True)
     sk_model = register_model(sk_model, SCIKIT_MODEL_NAME, PROJECT_NAME, input=X)
 
+    # Test overwriting model content
+    mr.add_model_content(sk_model, 'Your mother was a hamster!', 'insult.txt')
+    mr.add_model_content(sk_model, 'And your father smelt of elderberries!', 'insult.txt')
+
     # Publish to MAS
     sas_module = publish_model(sas_model, 'maslocal', replace=True)
     sk_module = publish_model(sk_model, 'maslocal', replace=True)

tests/cassettes/test_project_with_sas_and_sklearn_regression_models.test_swat.json

@@ -15,8 +15,13 @@ def test_len_no_paging():
     items = [{'name': 'a'}, {'name': 'b'}, {'name': 'c'}]
     obj = RestObj(items=items, count=len(items))
 
+    # PagedList should end up effectively identical to a standard list since no paging required.
+    target = [RestObj(x) for x in items]
+
     with mock.patch('sasctl.core.request') as request:
         l = PagedList(obj)
+        assert str(l) == str(target)
+        assert repr(l) == repr(target)
         assert len(l) == 3
 
         for i, o in enumerate(l):
@@ -82,9 +87,9 @@ def side_effect(_, link, **kwargs):
             if i < len(source_items) - 1:
                 # Ellipses should indicate unfetched results unless we're
                 # at the end of the list
-                assert str(l).endswith(', ... ]')
+                assert str(l).endswith(', ...]')
             else:
-                assert not str(l).endswith(', ... ]')
+                assert not str(l).endswith(', ...]')
 
 
 def test_getitem_paging(paging):
@@ -95,9 +100,16 @@ def test_getitem_paging(paging):
     # length of list should equal total # of items
     assert len(l) == len(items)
 
+    # If number of items on first page don't match total number of items then
+    # some paging is required, so repr() should contain elipses indicating more data.
+    if len(obj['items']) < obj.count:
+        assert str(l).endswith(', ...]')
+
     for i, item in enumerate(l):
         assert item.name == RestObj(items[i]).name
 
+    assert not str(l).endswith(', ...]')
+
 
 def test_get_item_inflated_len():
     """Test behavior when server overestimates the number of items available."""