feat: session and identity integrate with context now #264
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Major refactoring of the PostHog Python SDK to integrate session and identity management with the context system, improving state management and API consistency.
- Added new context-aware functions (
identify_context,set_context_session) inposthog/scopes.pyfor better session and identity handling - Introduced
ContextScopeclass with proper parent-child relationships, replacing simple stack-based dictionary approach - Modified core client methods (identify, capture, set) to automatically handle session IDs and use context-based distinct_id fallback
- Version bumped from 5.0.0 to 5.1.0 for these non-breaking feature additions
- Added comprehensive test coverage for new context functionality in
test_scopes.py
6 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
|
Plan is to release this, and then bump the dependency in posthog and add the middleware... this should mean we'll start associating all caught exceptions sessions and distinct ID's ✨ automatically ✨, once the frontend bumps it's SDK version. |
| if distinct_id is None: | ||
| distinct_id = get_context_distinct_id() |
There was a problem hiding this comment.
Why is the session data not being added here like it is in some of the earlier methods?
There was a problem hiding this comment.
Capture exception doesn't add the session id because it calls capture under the hood, which does. It only bothers grabbing the distinct id because it's is unique in that it auto-generates a distinct id if one isn't set, rather than throwing. To prevent that auto-generation, we have to try and grab a context distinct id up front.
| return {} | ||
|
|
||
|
|
||
| # NOTE: We should probably remove this function - the way to clear scope context |
There was a problem hiding this comment.
Should we just remove this and the above? Feels like the tagging stuff is still in flux and I'm not sure anyone is fully relying on it yet. Maybe fine to wait given we're doing a major version bump soon
There was a problem hiding this comment.
Was gonna wait for the major version bump, yeah
posthog/scopes.py
Outdated
| Identify the current context with a distinct ID, associating all events captured in this or | ||
| child contexts with the given distinct ID (unless identify_context is called again). This is overridden by | ||
| distinct id's passed directly to posthog.capture and related methods (identify, set etc). |
There was a problem hiding this comment.
If I start a new context with fresh=True will the identity still be set? Don't know if it should be but I'm guessing it isn't. Might be worth clarifying that somewhere for the user.
There was a problem hiding this comment.
Updated the comments around what fresh means in relation to sessions and distinct id's
Also bump version for release