-
Notifications
You must be signed in to change notification settings - Fork 53
fix: delete sentry integration #262
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
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.
PR Summary
Comprehensive removal of unmaintained Sentry integration from posthog-python SDK to prevent potential data quality issues and prepare for improved Django middleware.
- Removed entire
sentry_django_exampledirectory andposthog/sentrymodule, including all integration code and examples - Cleaned up dependencies in
pyproject.tomlby removing 'sentry' optional dependency group and related packages - Simplified
exception_utils.pyby removing Sentry-specific documentation while preserving core functionality - Removed Django Sentry integration documentation from README.md to prevent users from implementing unsupported patterns
13 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
| @@ -1,4 +1,4 @@ | |||
| VERSION = "4.10.0" | |||
| VERSION = "5.0.0" | |||
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 know this is technically a breaking change but seems like a pretty niche feature to necessitate a major upgrade for 🤔 Don't suppose there's anything we could bundle in as part of this change like we did for Node recently?
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.
Version numbers are free, they let you make them as high as you want, even the major ones 😉.
More seriously, the next breaking change I want to make to this SDK is to make it optional to pass a distinct ID, and that's too large a change to bundle in here IMO. I'm not aware of anyone else working on anything breaking, and I think "biting the bullet" of major version bump for a relatively niche feature here wins us some credibility about future breaking changes.
We don't support this mode of using posthog error tracking, so this code is ~entirely unmaintained. I'm fairly sure if anyone /did/ try to use it, it'd send invalid data to posthog, which our pipeline would drop. Tearing it out as part of general SDK improvement work, first step in improving our django middleware/integration offering.