LITE-24467 Fix slicing for ResourceSet #46
Conversation
bdjilka
force-pushed
the
LITE-24467_fix_slicing_for_resourceset
branch
2 times, most recently
from
e0c58d8 to
a3a7c8a
Compare
connect/client/models/iterators.py
Outdated
| raise | ||
| self._config['params']['offset'] += self._config['params']['limit'] | ||
| items_to_fetch = self._rs._slice.stop - self._rs._content_range.last - 1 | ||
| if items_to_fetch < self._config['params']['limit']: |
There was a problem hiding this comment.
Probably if items_to_fetch is 0 we can raise StopIteration instead of making the call or I'm wrong ?
There was a problem hiding this comment.
It will be, upper there is a condition self._rs._content_range.last >= self._rs._slice.stop - 1
connect/client/models/iterators.py
Outdated
| @@ -124,6 +124,58 @@ async def _execute_request(self): | |||
| return results, content_range | |||
|
|
|||
|
|
|||
| class AbstractSliceIterator(AbstractBaseIterator): | |||
There was a problem hiding this comment.
IMO this logic can be added to the AbstractIteration instead of having a new iterator class for slices, am I wrong ?
There was a problem hiding this comment.
AbstractIteration caches results, tests showed, that it leads to different error. Moreover, AbstractIteration don't have def __iter__(self):
connect/client/models/resourceset.py
Outdated
| copy._slice = key | ||
| if copy._slice.stop - copy._slice.start < copy._limit: | ||
| copy._limit = copy._slice.stop - copy._slice.start | ||
| raise NotYetEvaluatedError() |
There was a problem hiding this comment.
It's a bit complicated for me to merge having fixed just sync client, can you do the same for async one ?
There was a problem hiding this comment.
Yes, it is in progress
bdjilka
force-pushed
the
LITE-24467_fix_slicing_for_resourceset
branch
3 times, most recently
from
16b5859 to
7467c98
Compare
bdjilka
force-pushed
the
LITE-24467_fix_slicing_for_resourceset
branch
from
7467c98 to
cd1bd48
Compare
ghost
merged commit 
Now only for synchronous code. Async one will be later