Feature/expand sentry boilerplate#19
Conversation
|
Tools report for GitHub-mbdevpl/python-boilerplates/PR-19 [build #1]:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
========================================
- Coverage 100.0% 98.7% -1.3%
========================================
Files 17 17
Lines 1005 1021 +16
Branches 76 77 +1
========================================
+ Hits 1005 1008 +3
- Misses 0 12 +12
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| debug: bool = False | ||
| attach_stacktrace: bool = False | ||
| shutdown_timeout: float = 2.0 |
There was a problem hiding this comment.
https://docs.sentry.io/platforms/python/configuration/options/#shutdown_timeout
according to Sentry docs, the timeout variable type is int -- so not sure if specifying decimal places would even work.
There was a problem hiding this comment.
Yeah, the documentation is not in sync with the code on this one. It is actually float if you check the code type definition: https://github.com/getsentry/sentry-python/blob/adcfa0f6abf8850f3b007bde609d0f943f621786/sentry_sdk/consts.py#L506
| @classmethod | ||
| def remove_tags(cls, tags: t.List[str]): | ||
| """Remove tags from the current scope.""" | ||
| with sentry_sdk.configure_scope() as scope: |
There was a problem hiding this comment.
Maybe this method can be removed as it's not used or tested.
Is there any use case when one would like to remove the tags once they are set?
There was a problem hiding this comment.
I think there was justification for this method initially, but with the comment below, the wrapper class function does nothing more than calling the sentry function, at which point it is pretty redundant. So I have removed it.
| @classmethod | ||
| def set_tags(cls, tags: t.Dict[str, str]): | ||
| """Set tags for the current scope.""" | ||
| with sentry_sdk.configure_scope() as scope: | ||
| scope.set_tags(tags) |
There was a problem hiding this comment.
https://docs.sentry.io/platforms/python/migration/1.x-to-2.x#scope-configuring
According to Sentry docs, the sentry_sdk.configure_scope is deprecated -- also, I think if there's no specific scope here which we're creating, and Sentry docs suggest in such case to use global-level API methods, see here: https://docs.sentry.io/platforms/python/enriching-events/scopes/
I wonder if the code for setting tags could be added to the init method directly, in something like (in init() method directly) would suffice. Or even just set_tags without the if as the default value is empty dict.
if cls.tags:
sentry_sdk.set_tags(cls.tags)
There was a problem hiding this comment.
Thanks! I removed the configure scope option, and seeing as the function just becomes a wrapper of another function, I removed the tag handling class functions.
I still set tags in init. I check that it would be safe to just feed the cls.tags variable with default {}, that will result in no-op.
|
@StianHanssen Unfortunately, seems that CI is not cooperating on this contributed PR and I can't easily fix it. So, I've made a refactored version of this PR that includes all changes from here, plus some extras like adding you as a contributor. I'm closing this, please see the refactored PR: |
Changes made: