Skip to content

Improve pagination handling in Confluence API client #3321

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve pagination handling in Confluence API client #3321

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f37bb0
Improve pagination handling in Confluence API client and enhance erro…
WildDogOne Mar 22, 2025
983a68d
Merge branch 'main' into main
seanstory Mar 28, 2025
db10dda
Update connectors/sources/confluence.py
WildDogOne Mar 31, 2025
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
25 changes: 16 additions & 9 deletions connectors/sources/confluence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,31 @@ async def paginated_api_call(self, url_name, **url_kwargs):
Yields:
response: JSON response.
"""
url = os.path.join(self.host_url, URLS[url_name].format(**url_kwargs))
base_url = os.path.join(self.host_url, URLS[url_name].format(**url_kwargs))
start = 0

while True:
try:
url = f"{base_url}&start={start}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line probably needs to be outside of the while True, otherwise it'll overwrite the last iteration's attempts to set the URL to the "next" link, if that strategy was used.

self._logger.debug(f"Starting pagination for API endpoint {url}")
response = await self.api_call(url=url)
json_response = await response.json()

links = json_response.get("_links")
yield json_response
if links.get("next") is None:
if links.get("next"):
url = os.path.join(
self.host_url,
links.get("next")[1:],
)
elif json_response.get("start") + json_response.get("size") < json_response.get("totalSize"):
start = json_response.get("start") + json_response.get("size")
url = f"{base_url}&start={start}"
else:
return
url = os.path.join(
self.host_url,
links.get("next")[1:],
)
except Exception as exception:
self._logger.warning(
f"Skipping data for type {url_name} from {url}. Exception: {exception}."
f"Skipping data for type {url_name} from {base_url}. Exception: {exception}."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd actually still want to log out url, since it would let us know which "page" of results had an issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only reason I switched this to base_url is because it's possible that url does not exist if try step fails (or at least so I think it could happen)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Ok, hadn't caught this, but I think url needs to be defined outside of the loop to start. That way we can reference it here, and we also can be sure that it's not re-set on each iteration (see my most recent comment)

)
break

Expand Down Expand Up @@ -984,8 +992,7 @@ async def search_by_query(self, query):
# entity can be space or content
entity_details = entity.get(SPACE) or entity.get(CONTENT)

if (
entity_details.get("type", "") == "attachment"
if not entity_details or (entity_details.get("type", "") == "attachment"
and entity_details.get("container", {}).get("title") is None
):
continue
Expand Down