Skip to content

Add optional retry for transient CensusGeocoder failures - #1940

Open
JakeSelby wants to merge 1 commit into
move-coop:mainfrom
JakeSelby:geocode-retry-transient
Open

JakeSelby wants to merge 1 commit into
move-coop:mainfrom
JakeSelby:geocode-retry-transient

Conversation

@JakeSelby

Copy link
Copy Markdown
Contributor

What is this change?

  • censusgeocode re-raises RequestException and CensusGeocoder never caught it, so a single
    connection reset or timeout ended the whole call. For geocode_address_batch that means a long
    job dies partway through its chunks.
  • Adds a retries constructor argument and a _request helper that retries connection-level
    failures with 2 ** attempt backoff. The retry wraps the per-chunk call in
    geocode_address_batch, so completed chunks are never re-sent.
  • Fixes [Feature/Addition] Optional retry for transient CensusGeocoder request failures #1931.

Considerations for discussion

  • The default is 0, which preserves today's behavior exactly. Retrying by default would
    change request timing for every existing user and could multiply load on a government endpoint,
    so it seemed wrong to opt everyone in. Newmode defaults to 2 retries if you would rather this
    matched that.
  • Only RequestException is retried, and that is a real limit worth naming. censusgeocode
    never calls raise_for_status(), so the Census API's own error responses do not raise: a 5xx
    reaches _fetch as a JSON decode failure and surfaces as
    ValueError("Unable to parse response from Census"), which is indistinguishable from a genuine
    parse problem. Retrying ValueError would therefore also retry real bugs, so I left it alone.
    Making HTTP status failures visible is a separate change and I have filed it separately.
  • Put on the constructor rather than each method, matching Redshift, Postgres, MySQL and
    Braintree.

How to test the changes (if needed)

uv run --no-default-groups --group test --extra geocode pytest test/test_geocode

Four new tests cover recovery after transient failures, re-raising once retries are exhausted,
the no-retry default, and that all four public methods are wrapped.

Breaking Changes

Breaking changes are changes to our public API which may require existing users to change their code. If there are no breaking changes, any existing parsons user should not need to do anything after updating their parsons version.

Does this PR introduce breaking changes?
  • label: Breaking change — This PR introduces one or more breaking changes.
  • label: Non-breaking change — This PR does not introduce one or more breaking changes.

Details (if needed)

  • New optional keyword argument defaulting to 0, which makes exactly one attempt and never
    sleeps -- identical to current behavior. No signature or return-type change.

censusgeocode re-raises RequestException and CensusGeocoder did not
catch it, so a connection reset or a timeout ended the whole call --
including a batch job partway through its chunks.

Adds a retries constructor argument and a _request helper that retries
connection-level failures with 2 ** attempt backoff. The retry wraps the
per-chunk call in geocode_address_batch, so completed chunks are not
re-sent. The default of 0 preserves today's behavior exactly, and a
negative value is rejected rather than silently skipping the request.

Only RequestException is retried. The Census API's own error statuses do
not raise -- censusgeocode never calls raise_for_status -- so they are
out of scope here.
@github-actions github-actions Bot added testing Work type - writing or changing code tests for core Parsons features or Parsons connectors python Pull requests that update Python code non-breaking-change Status - Indicates that the code in this PR does not have any breaking changes. labels Aug 28, 2026
Comment on lines +45 to +46
def _request(self, func, *args, **kwargs):
# Retries transient network failures only; see the class docstring for the limits.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def _request(self, func, *args, **kwargs):
# Retries transient network failures only; see the class docstring for the limits.
def _request(self, func, *args, **kwargs):
"""
Facilitate retrying requests when network failures occur.
Failed requests resulting in retries will be logged.
See class docstring for more information.
Args:
func (Callable): function to wrap with retry logic
Returns:
Value returned by provided callable
"""

Doesn't have to be exactly that, but a docstring here would be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

non-breaking-change Status - Indicates that the code in this PR does not have any breaking changes. python Pull requests that update Python code testing Work type - writing or changing code tests for core Parsons features or Parsons connectors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature/Addition] Optional retry for transient CensusGeocoder request failures

2 participants