-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix frequent LookupError exceptions #282
base: master
Are you sure you want to change the base?
Conversation
ports: | ||
- 6379:6379 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can I ask what this change does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually nvm I just read your commit messages which answered my question
@@ -338,7 +338,7 @@ def sort_dtos_by_date(input_dtos): | |||
|
|||
def log_to_sentry(message, extra=None, level="info"): | |||
extra = extra or {} | |||
with sentry_sdk.new_scope() as scope: | |||
with sentry_sdk.push_scope() as scope: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this is what we had before (https://github.com/uktrade/lite-hmrc/pull/274/files) so am I right to infer that the main change for this PR that we think will solve the issue is the version downgrade to 1.x ?
This change attempts to fix the LookupError exceptions which we have been seeing frequently on sentry for production lite-hmrc. Unfortunately, this particular exception is very tricky to reproduce - so it's difficult to know if this will fix it or not. This PR takes a bit of an educated guess that the LookupErrors started when we upgraded sentry-sdk to 2.X; looking at the errors it seems there are issues accessing the
ContextVar
current_scope
and it seems from looking at the sentry-sdk codebase that there has been work around this between 1.X and 2.X.We don't have the luxury of time to look too deeply in to this - as it's not a blocking error and just an annoyance.