feat(data-warehouse): mark DoIt "Report no longer exists" as non-retryable#60816
Conversation
…yable The DoIt source raises "Report no longer exists" when the configured report ID can't be resolved (e.g. the report was deleted or renamed in DoIt). Retrying this never succeeds, so register it as a non-retryable error to disable the source instead of looping through retries, and surface an actionable message to the user. Generated-By: PostHog Code Task-Id: 94798ff3-45ae-48d9-b38b-22d6760068c9
|
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 |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
posthog/temporal/data_imports/sources/doit/tests/test_doit_source.py:13-15
The repository convention for tests like this is to use `@pytest.mark.parametrize`, as seen in `test_attio_source.py` and similar files. Using the parametrized form here keeps the pattern consistent and makes it easy to add more error keys in the future without restructuring the test.
```suggestion
@pytest.mark.parametrize("pattern", ["Report no longer exists"])
def test_non_retryable_errors_includes_pattern(self, pattern):
errors = self.source.get_non_retryable_errors()
assert pattern in errors
```
Reviews (1): Last reviewed commit: "feat(data-warehouse): mark DoIt "Report ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR updates the DoIt data warehouse source to treat the permanent DoIt API error "Report no longer exists" as non-retryable, so failed imports can be disabled and surfaced to users with a more actionable message rather than consuming retries indefinitely.
Changes:
- Added a
DoItSource.get_non_retryable_errors()override registering"Report no longer exists"with a friendly user-facing message. - Added a small unit test asserting the DoIt source type and that the non-retryable pattern is registered.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
posthog/temporal/data_imports/sources/doit/source.py |
Registers the DoIt “Report no longer exists” error as non-retryable with a friendly message. |
posthog/temporal/data_imports/sources/doit/tests/test_doit_source.py |
Adds a unit test ensuring the non-retryable error key is present and the source type is DoIt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow the repo convention (as in test_attio_source.py) of using @pytest.mark.parametrize for non-retryable error pattern assertions, so adding more patterns later doesn't require restructuring the test. Generated-By: PostHog Code Task-Id: 94798ff3-45ae-48d9-b38b-22d6760068c9
Problem
The DoIt data warehouse source raises
Report no longer existswhen the configured report ID can't be resolved from DoIt's reports list — typically because the report was deleted or renamed in DoIt. This error is permanent: retrying the import will never succeed, so the source keeps burning retry attempts on a failure that can only be resolved by user action.Changes
Registered
Report no longer existsas a non-retryable error onDoItSourceviaget_non_retryable_errors(). When the import activity hits this error (substring match), it now disables the source instead of retrying, and surfaces an actionable message telling the user to reconnect the source or pick a different report.Added a small unit test covering the source type and the presence of the non-retryable error key.
This mirrors the existing pattern used by other sources (e.g. Stripe).
How did you test this code?
I'm an agent (Claude Code). I added an automated unit test (
posthog/temporal/data_imports/sources/doit/tests/test_doit_source.py) asserting the source type and that the non-retryable error is registered. I did not run the test locally — the Python environment isn't bootstrapped in this sandbox — so CI is the source of truth for execution.Automatic notifications
Docs update
No docs changes needed.
🤖 Agent context
Authored by Claude Code at the request of the repo owner. The change follows the established
get_non_retryable_errors()convention; I confirmed the import activity does substring matching against the raised exception text ("Report no longer exists"raised indoit.py), so the key matches as-is. The friendly value message was added since the raw exception text is terse. No alternative approaches were rejected — this is the standard pattern across warehouse sources.