-
Notifications
You must be signed in to change notification settings - Fork 28
feat(insights): event_context #214
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
Conversation
This now uses the ContextVar and a helper ContextStore class.
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.
Pull Request Overview
Adds a new event context API and refactors how context is stored using ContextVar
-backed stores.
- Introduce
ContextStore
for thread- and async-safe context management - Add event context methods (
set_event_context
,reset_event_context
,event_context
) and merge them inHoneybadger.event
- Refactor
Notice
to remove thread locals and improve tag parsing
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
pytest.ini | Configure default fixture loop scope for asyncio tests |
dev-requirements.txt | Add pytest-asyncio |
honeybadger/context_store.py | New ContextStore using ContextVar |
honeybadger/core.py | Replace threadlocals with ContextStore ; add event-context APIs |
honeybadger/notice.py | Remove thread-local context processing; enhance tag parsing |
honeybadger/protocols.py | Rename protocol parameter from payload to notice |
honeybadger/connection.py | Remove unused Protocol import |
honeybadger/types.py | Clean up unused imports |
honeybadger/tests/test_core.py | Add tests for async/thread isolation and event-context behavior |
honeybadger/tests/test_notice.py | Update notice-tag tests |
honeybadger/tests/contrib/test_asgi.py | Adjust ASGI plugin tests for new event-context handling |
Comments suppressed due to low confidence (3)
honeybadger/tests/contrib/test_asgi.py:6
- [nitpick] Importing
honeybadger
this way is ambiguous; consider importing the module or class explicitly (e.g.import honeybadger as hb
orfrom honeybadger import Honeybadger
) to clarify what’s being referenced.
from honeybadger import honeybadger
honeybadger/notice.py:55
- The
_get_thread_context
method is no longer used after removing thread-local support; consider removing this dead code to keep the class focused.
def _get_thread_context(self):
honeybadger/protocols.py:6
- [nitpick] The parameter name was changed from
payload
tonotice
; update any docstrings or inline comments to reflect this rename for clarity.
def send_notice(self, config: Any, notice: Notice) -> Optional[str]:
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.
This looks good, I do like how this simplifies the Notice class. There is one more Notice
class initialization that needs to be updated here.
Great catch @roelbondoc ! |
This adds a new set of functions to the
Honeybadger
class for setting and applying event context.I also refactored the internals of storing context quite a bit:
ContextStore
class that usesContextVar
, which is the recommended way to manage thread local variables in a 3.7+ world.ContextStore
to create both notice and event context stores