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

fix: handle 0 item response in querysets #1501

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

jorwoods
Copy link
Contributor

@jorwoods jorwoods commented Oct 15, 2024

A flaw in the __iter__ logic introduced to handle scenarios where a pagination element is not included in the response xml resulted in an infinite loop. This PR introduces a few changes to protect against this:

  1. After running QuerySet._fetch_all(), if the result_cache is empty, return instead of performing other comparisons.
  2. Ensure that any non-None total_available is returned from the PaginationItem's object.
  3. In _fetch_all, check if there is a PaginationItem that has been populated so as to not call the server side endpoint multiple times before returning.

A flaw in the __iter__ logic introduced to handle scenarios
where a pagination element is not included in the response xml
resulted in an infinite loop. This PR introduces a few changes
to protect against this:

1. After running QuerySet._fetch_all(), if the result_cache is
empty, return instead of performing other comparisons.
2. Ensure that any non-None total_available is returned from
the PaginationItem's object.
3. In _fetch_all, check if there is a PaginationItem that has been
populated so as to not call the server side endpoint muliple times
before returning.
Tests were failing because the fetch_all method added a second
check before fetching the next page. This fix will allow the
next page to be retrieved when used normally
@jacalata jacalata merged commit 2ff9697 into tableau:development Oct 17, 2024
22 checks passed
@jorwoods jorwoods deleted the jorwoods/infinite_loop branch October 17, 2024 17:38
jacalata pushed a commit that referenced this pull request Oct 24, 2024
* Feature: export custom views #999 (#1506)
* feat(exceptions): separate failed signin error (#1478)
* refactor request_options, add language param (#1481)
* Set FILESIZE_LIMIT_MB via environment variables (#1466)
* added PulseMetricDefine cap (#1490)
* Adding project permissions handling for databases, tables and virtual connections (#1482)
* fix: queryset support for flowruns (#1460)
* fix: set unknown size to sys.maxsize
* fix: handle 0 item response in querysets (#1501)
* chore: support VizqlDataApiAccess capability (#1504)
* refactor(test): extract error factory to _utils
* chore(typing): flowruns.cancel can also accept a FlowRunItem
* chore: type hint default permissions endpoints (#1493)
* chore(versions): update remaining f-strings (#1477)
* Make urllib3 dependency more flexible (#1468)
* Update requests library for CVE CVE-2024-35195 (#1507)

* chore(versions): Upgrade minimum python version (#1465)
* ci: cache dependencies for faster builds (#1497)
* ci: build on python 3.13 (#1492)
* Update samples for Python 3.x compatibility (#1479)
* chore: remove  py2 holdover code (#1496)
* #Add 'description' to datasource sample code (#1475)
* Remove sample code showing group name encoding (#1486)
* chore(typing): include samples in type checks (#1455)

* fix: docstring on QuerySet
* docs: add docstrings to auth objects and endpoints (#1484)
* docs: docstrings for Server and ServerInfo (#1494)
* docs: docstrings for user item and endpoint (#1485)
* docs: docstrings for site item and endpoint (#1495)
* docs: workbook docstrings (#1488)
* #1464 - docs update for filtering on boolean values (#1471)

---------
Co-authored-by: Brian Cantoni <[email protected]>
Co-authored-by: Jordan Woods <[email protected]>
Co-authored-by: Jordan Woods <[email protected]>
Co-authored-by: Jac <[email protected]>
Co-authored-by: Henning Merklinger <[email protected]>
Co-authored-by: AlbertWangXu <[email protected]>
Co-authored-by: TrimPeachu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants