Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Dumps: Add Sentry error logging #6319

Merged
merged 6 commits into from
Mar 25, 2022

Conversation

cclauss
Copy link
Collaborator

@cclauss cclauss commented Mar 20, 2022

As discussed in several issues related to #5402 our data dumps can be quite tricky so this PR logs data dump error output into Sentry to ease debugging.

Technical

Testing

See the cron workflow at #5892

Screenshot

Stakeholders

@cdrini
Copy link
Collaborator

cdrini commented Mar 24, 2022

Chatted on call:

Note we might need to pass in the path to the openlibrary.yml; I can't recall if importing it this way does the magic for local/prod environment.

@cclauss cclauss marked this pull request as ready for review March 24, 2022 21:05
@cclauss cclauss changed the title DRAFT: Data Dumps: Add Sentry error logging Data Dumps: Add Sentry error logging Mar 24, 2022
openlibrary/data/dump.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Code lgtm (excluding that typo :P )! Can you try testing it by running the dump.py script locally, and copying the dsn into your local openlibrary.yml? Should see first event appear in sentry UI :)

Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Tested on call with Chris running it locally with correct DSN; appeared in sentry UI 👍

Need to test a full data dump as well later

@cdrini cdrini merged commit 489e6d3 into internetarchive:master Mar 25, 2022
@cclauss cclauss deleted the add-sentry-to-data-dumps branch March 25, 2022 22:09
@cdrini cdrini self-assigned this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants