fix(taboola): retry the initial token mint on transient errors - #76422
Conversation
The initial access-token mint in `get_rows` ran before the `@retry`-decorated `fetch` closure existed, so a transient 429/5xx from the token endpoint raised `TaboolaRetryableError` straight out of the sync activity instead of backing off, even though the mid-sync re-mint on a 401 already benefited from that retry. Wrap both call sites in the same retry policy via a shared `mint_token` closure. Generated-By: PostHog Code Task-Id: 34f1efbd-102b-40cc-a9a6-655f9adb71b3
|
😎 Merged successfully - details. |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
There was a problem hiding this comment.
Pull request overview
This PR hardens the Taboola warehouse source sync by ensuring the initial OAuth token mint is covered by the same transient-error retry policy already applied to API fetching, preventing TaboolaRetryableError (e.g. token endpoint 503) from escaping the sync activity immediately.
Changes:
- Added a
mint_token()closure inget_rowsdecorated with tenacity retry onTaboolaRetryableError. - Routed both the initial token mint and the mid-sync re-mint (after a 401) through
mint_token(). - Added a regression test ensuring the initial token mint retries on a transient 503 and then succeeds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| products/warehouse_sources/backend/temporal/data_imports/sources/taboola/taboola.py | Adds a shared tenacity-retried token mint closure and uses it for initial + 401 re-mints. |
| products/warehouse_sources/backend/temporal/data_imports/sources/taboola/tests/test_taboola.py | Adds a test that simulates a transient token endpoint failure on the first mint and validates retry behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…fetch Only the initial token mint needs its own retry decorator — it's the one call site nothing else covers. The mid-sync re-mint on a 401 already propagates into fetch's own retry, so decorating it too let both retries stack (up to 25 attempts in the worst case). Address Copilot review feedback on the PR. Generated-By: PostHog Code Task-Id: 34f1efbd-102b-40cc-a9a6-655f9adb71b3
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Small, contained retry-logic fix in a warehouse source connector by an owning-team author, with a regression test and the reviewer's compounding-retry concern already resolved in a follow-up commit.
- 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 20L, 1F substantive, 34L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (34L, 2F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ f381e58 · reviewed head 45268ed |
|
/trunk merge |
Problem
Error tracking surfaced a
TaboolaRetryableError("Taboola token endpoint error (retryable): status=503") escaping a Taboola data warehouse sync, raised from_mint_tokeninproducts/warehouse_sources/backend/temporal/data_imports/sources/taboola/taboola.py.Looking at the call sites:
get_rowsmints an access token once up front, then defines afetchhelper wrapped in atenacityretry (covering rate limits and transient 5xx) that re-mints the token if it expires mid-sync (401). The up-front mint happens before that retry wrapper exists, so a transient failure from the token endpoint at that point raised straight out of the activity instead of backing off — even though the code's own comment says these failures "must be retryable."Changes
mint_tokenclosure decorated with the same retry policy already used forfetch(retry onTaboolaRetryableError, capped attempts, exponential jitter backoff).This is a fragile-code fix rather than a user/upstream error — the request shape/config is fine, our own retry coverage just had a gap.
How did you test this code?
Added
test_initial_token_mint_retries_on_transient_errortotest_taboola.py, which returns a 503 on the first token POST and a valid token on the second. Verified it fails without the fix (raisesTaboolaRetryableErrorimmediately) and passes with it (retries and succeeds), so it locks in the fix.Ran the full taboola test suite (
test_taboola.py+test_taboola_source.py, 40 tests) — all pass. Ranruff check --fixandruff format --diff— clean. Ranmypyagainst the changed file — no issues.Automatic notifications
Docs update
N/A — internal retry-handling fix, no user-facing behavior or documented workflow changed.
🤖 Agent context
Autonomy: Fully autonomous
I (an agent) triaged a Taboola error-tracking issue reporting
TaboolaRetryableError: Taboola token endpoint error (retryable): status=503. I pulled the issue's stack trace and confirmed it originated in this source's own_mint_token/get_rowscode, read the implementation to find the retry gap described above, and applied the minimal fix plus a regression test that fails without it. No skills beyond the repo's testing conventions were needed for a change this scoped.