fix: surface real error cause and make round-robin summary visible in prod#4
Open
ttomalak-bkpr wants to merge 1 commit into
Open
fix: surface real error cause and make round-robin summary visible in prod#4ttomalak-bkpr wants to merge 1 commit into
ttomalak-bkpr wants to merge 1 commit into
Conversation
… prod GitHubRestStream.get_records now logs the actual exception (type + message) before re-raising - singer_sdk's own catch-all only logs "an unhandled error occurred while syncing '<stream>'" with no detail, which left prod runs with no way to tell why a stream died (rate limit, auth, etc). The round-robin checkpoint's per-run summary was logged at info, but prod runs at warning - so it has never actually appeared in production logs since it was added. Bumped to warning so enumeration progress is visible without needing a log-level change.
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
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.
Summary
GitHubRestStream.get_recordsnow logs the real exception (type + message) before re-raising. singer_sdk's own catch-all only logs"an unhandled error occurred while syncing '<stream>'"with zero detail, so prod runs currently give no way to tell why a stream died (rate limit, auth, transient API error, ...).FatalAPIError/RetriableAPIErroralready carry a rich message (HTTP status, GitHub-Request-Id, etc.) - this just makes sure it isn't lost.RepositoryStream._get_records_round_robin) was logged atinfo, but prod actually runs atwarning- so despite already being deployed, this summary has never appeared in production logs. Bumped both the normal summary and the "0 repos returned" early-return towarning, so enumeration progress (repos processed, full lap or not, next resume point) is finally visible without any deploy-side log-level change.This is scoped entirely to this repo - no changes to the
bi-meltanolauncher/image that runs it.Test plan
ruff check/ruff format --checkon both changed files - clean (pre-existing unrelated line-length issues onrepository_streams.pyleft untouched, confirmed present onmastertoo)mypyon both changed files - cleanpytest tests/) - same 72 passed / 10 pre-existing network-dependent failures as onmaster, no regressions