fix(data-imports): handle sources without schema discovery in database_schema - #68114
Merged
Gilbert09 merged 2 commits intoJul 3, 2026
Merged
Conversation
…e_schema The database_schema endpoint called source.get_schemas() inside a broad try/except that ran every non-classified exception through capture_exception. Unreleased sources inherit the base get_schemas, which raises a bare NotImplementedError, so hitting the endpoint for one surfaced as captured error-tracking noise instead of an actionable response. Add an except NotImplementedError branch that returns a clean 400 without capturing, mirroring the existing guard in the sibling setup endpoint. Generated-By: PostHog Code Task-Id: 3404976d-6fb3-4da8-8559-e268cc3f0e85
Contributor
|
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 |
Contributor
|
Reviews (1): Last reviewed commit: "fix(data-imports): handle sources withou..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…test Switch the database_schema NotImplementedError test from the unreleased AmazonCloudWatch source to AmazonS3, whose get_schemas omission is deliberate and stable. Mirrors the sibling setup test and avoids silently losing branch coverage if an unreleased source later implements schema discovery. Generated-By: PostHog Code Task-Id: 3404976d-6fb3-4da8-8559-e268cc3f0e85
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Gilbert09
deleted the
posthog-code/database-schema-notimplemented-graceful
branch
July 3, 2026 08:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Error tracking surfaced a
NotImplementedErrorwith no message, captured as an unexpected server error, coming out of the data-warehouse import pipeline:get_schemasatproducts/warehouse_sources/backend/temporal/data_imports/sources/common/base.pydatabase_schemaAPI endpoint (ExternalDataSourceViewSet.database_schema)Issue: https://us.posthog.com/project/2/error_tracking/019f261a-ca75-75a3-9837-182e94289ce6
The base
_BaseSource.get_schemasraises a bareNotImplementedErrorfor any source that doesn't implement schema discovery (unreleased stubs that inheritSimpleSourcedirectly, e.g. Amazon CloudWatch). Thedatabase_schemaendpoint wrapped theget_schemascall in a broadexcept Exception, and since a bareNotImplementedErrormatches none of the expected-error patterns in_classify_refresh_schemas_error, it fell through tocapture_exceptionand returned the generic "Could not fetch schemas from source." message.This is our code being fragile, not a user credential or upstream problem. There is nothing to retry — the source simply has no schema discovery — so it should degrade gracefully rather than page us.
Changes
Add an
except NotImplementedErrorbranch todatabase_schemathat returns a clean400with a clear message and does not capture the exception. This mirrors the guard the siblingsetupendpoint already has for the same baseNotImplementedError, so the two schema-discovery entry points now behave consistently.No behavior change for sources that implement
get_schemas.How did you test this code?
Added
test_database_schema_rejects_source_without_schema_discovery, which posts a source type that inherits the baseget_schemasto the endpoint and asserts a400with the friendly message and thatcapture_exceptionis not called. Before this change that path captured theNotImplementedErrorand returned the generic fallback message. No existing test covereddatabase_schemafor a source without schema discovery — the sibling case (test_setup_rejects_source_without_schema_discovery) only exercises thesetupendpoint, andtest_database_schema_captures_only_unexpected_source_errorsmocksget_schemasrather than raising the real base error.I (Claude) could not run the Django test suite in this environment — pytest's backing databases are not provisioned here (the pre-existing sibling test fails the same way on connection). I ran
ruff checkandruff format --checkover both touched files (both clean), and statically verified the source hierarchy:AmazonCloudWatchSourceinheritsSimpleSourcewithout overridingget_schemas, so the base raisesNotImplementedError. The new test mirrors the already-passingsetupguard test.🤖 Agent context
Autonomy: Fully autonomous
I (Claude) found this while triaging an error-tracking issue in the data-warehouse import pipeline. I confirmed the issue via the PostHog error-tracking tools (full stack trace, single occurrence, top in-app frame at
base.pyget_schemas), traced the call path fromdatabase_schemainto the shared base source, and classified it as a fixable fragility rather than a user/upstream error worth adding toNonRetryableErrors.I invoked
/improving-drf-endpointsand/writing-testswhile shaping the change. I chose the view-layer fix (rather than touching the shared base or the classifier) because thesetupendpoint already establishes exactly this pattern for the same exception, so mirroring it keeps the two entry points consistent and the change obviously correct. I checked open PRs (including the maintainer's own) for duplicates: #68098 guards the same endpoint but for a different root cause (unknown source type raisingValueError), and #67605 touches thesetuppath and MCP classification, not thisget_schemascall site.