Skip to content

Update wbi_helpers.py #834

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 1 commit into
base: master
Choose a base branch
from
Open

Update wbi_helpers.py #834

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 10 additions & 14 deletions wikibaseintegrator/wbi_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def mediawiki_api_call_helper(data: dict[str, Any], login: _Login | None = None,


@wbi_backoff()
def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | None = None, user_agent: str | None = None, max_retries: int = 1000, retry_after: int = 60) -> dict[
def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | None = None, user_agent: str | None = None, retry_after: int = 60) -> dict[
str, dict]:
"""
Static method which can be used to execute any SPARQL query
Expand All @@ -231,7 +231,6 @@ def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str |
:param query: The actual SPARQL query string
:param endpoint: The URL string for the SPARQL endpoint. Default is the URL for the Wikidata SPARQL endpoint
:param user_agent: Set a user agent string for the HTTP header to let the Query Service know who you are.
:param max_retries: The number time this function should retry in case of header reports.
:param retry_after: the number of seconds should wait upon receiving either an error code or the Query Service is not reachable.
:return: The results of the query are returned in JSON format
"""
Expand Down Expand Up @@ -260,31 +259,28 @@ def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str |

log.debug("%s%s%s", BColors.WARNING, params['query'], BColors.ENDC)

for _ in range(max_retries):
try:
response = helpers_session.post(sparql_endpoint_url, params=params, headers=headers)
except requests.exceptions.ConnectionError as e:
log.exception("Connection error: %s. Sleeping for %d seconds.", e, retry_after)

try:
response = helpers_session.post(sparql_endpoint_url, params=params, headers=headers)
except BaseException as e:
if config['BACKOFF_MAX_TRIES'] > 1:
sleep(retry_after)
continue
raise e
Comment on lines +265 to +268
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The sleep operation defeats the purpose of using the backoff decorator. The backoff decorator should handle all timing delays, so this manual sleep should be removed to let backoff manage retry timing properly.

Copilot uses AI. Check for mistakes.
else:
if response.status_code in (500, 502, 503, 504):
Comment on lines +263 to 270
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Catching BaseException is overly broad and will catch system exit signals like KeyboardInterrupt and SystemExit. This should be Exception instead to avoid interfering with system-level exceptions.

Copilot uses AI. Check for mistakes.
log.error("Service unavailable (HTTP Code %d). Sleeping for %d seconds.", response.status_code, retry_after)
sleep(retry_after)
continue
raise Exception("Service unavailable (HTTP Code %d)." % (response.status_code))
if response.status_code == 429:
if 'retry-after' in response.headers.keys():
retry_after = int(response.headers['retry-after'])
log.error("Too Many Requests (429). Sleeping for %d seconds", retry_after)
sleep(retry_after)
continue
response.raise_for_status()
raise Exception("Too Many Requests (429).")
Comment on lines +273 to +279
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Using generic Exception reduces error handling precision. Consider using a more specific exception type like requests.HTTPError or creating a custom exception class for HTTP service errors.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Using generic Exception reduces error handling precision. Consider using a more specific exception type like requests.HTTPError or creating a custom exception class for rate limiting errors.

Copilot uses AI. Check for mistakes.
Comment on lines +266 to +279
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Manual sleep calls before raising exceptions will cause double delays when combined with the backoff decorator. The backoff decorator should handle all retry timing, so these sleep calls should be removed.

Copilot uses AI. Check for mistakes.
results = response.json()

return results

raise Exception(f"No result after {max_retries} retries.")


def edit_entity(data: dict, id: str | None = None, type: str | None = None, baserevid: int | None = None, summary: str | None = None, clear: bool = False, is_bot: bool = False,
tags: list[str] | None = None, site: str | None = None, title: str | None = None, **kwargs: Any) -> dict:
"""
Expand Down