Feature/cursor paging #51
Conversation
|
|
||
|
|
||
| def get(self, limit: Optional[int] = None, *, params: Optional[dict] = None, **kwargs) -> List[dict]: | ||
| def get(self, url: Optional[str] = None, limit: Optional[int] = None, *, params: Optional[dict] = None, **kwargs) -> requests.Response: |
There was a problem hiding this comment.
We should keep the same List[dict] output as the current code.
Could we instead allow the token argument to be passed into this method?
There was a problem hiding this comment.
Option 1: Call session.get_response() separately in get and get_pages. (decouple)
Option 2: Pass token as an optional argument to get. If passed, change output type from List[dict] to Union[List[dict], Tuple[List[dict], Optional[str]]].
| params: Optional[dict] = None, # Optional alternative params | ||
| page_size: int = 100, | ||
| reverse_paging: bool = True, | ||
| reverse_paging: bool = False, |
There was a problem hiding this comment.
A note that some of these arguments are mutually-exclusive:
Default pagination:
reverse_paging
step_change_version
change_version_step_size
Curser-based pagination:
cursor_paging
partitioning
number
page_size can appear in both.
|
|
||
|
|
||
| elif cursor_paging and partitioning: | ||
| ods_version = tuple(map(int, self.client.get_ods_version().split(".")[:2])) |
There was a problem hiding this comment.
We should move the version-check to the top of the method and fail immediately if any disallowed arguments are passed.
| token = result.headers.get("Next-Page-Token") | ||
| return results | ||
|
|
||
| results = [element for sublist in Parallel(n_jobs=len(paged_tokens), backend="threading")(delayed(partitioning_with_token)(token) for token in paged_tokens) for element in sublist] |
There was a problem hiding this comment.
I'll need to dive deeper into this approach. There is a future where we enable async in the edfi_api_client and will need this method to be compatible.
No description provided.