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

Completely disable logging of warnings for unthumbnailable word docs, etc #2049

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

mbarnett
Copy link
Contributor

@mbarnett mbarnett commented Dec 18, 2020

Context

See #2046

After looking at this some more, and reading Rollbar's docs around error filtering more closely, it looks like #2046 isn't enough. We would keep sending this to Rollbar anyways, because a direct call like

Rollbar.warn("#{logo.record_type} with id: #{logo.record_id} and thumbnail #{logo.name} threw an error.", e)

will bypass the filters set in the Rollbar initializer.

What's New

Better to just disable this kind of logging entirely (also pulled it out of the logger since it's a waste of disk space to log every time someone sees a word doc or excel spreadsheet in a search result).

murny
murny previously approved these changes Jan 11, 2021
@ualbertalib-bot
Copy link

1 Error
🚫 The following CHANGELOG entries are missing a link to a PR/issue:
– Remove entirely unnecessary config file.
– Turn off reporting things like "this excel spreadsheet isn't thumbnailable" as warnings to Rollbar
– Completely disable logging of warnings around the "excel spreadsheet" issue

Generated by 🚫 Danger

Copy link
Contributor

@murny murny left a comment

Choose a reason for hiding this comment

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

👍

@mbarnett mbarnett merged commit 5899038 into master Feb 2, 2021
@mbarnett mbarnett deleted the msb/disable_thumbnail_warnings branch February 2, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants