Skip to content

Consider making Transporter abstract with all methods returning NotImplementedError #1803

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

Closed
DanielNoord opened this issue Dec 23, 2022 · 2 comments

Comments

@DanielNoord
Copy link

Problem Statement

We used multiple transporter to send to different DSN, this was the previously recommended way to solve this:
#198 (comment)

The new suggestion (#1521) doesn't really work for us as it would be infeasible to start contextmanagers everywhere we use the separate loggers.

However, bumping our sentry-sdk has introduced subtle issues as our TransporterProxy doesn't implement all methods.
Previously all methods were "abstract" but with a6a1be3 they no longer are.

I am asking to make the transporter abstract so that we get better warnings to implement new methods when they get added. I think other users will also benefit from this as it seems like a common use case and users might have followed the same recommendation.

Solution Brainstorm

I am open to making the PR myself but I'd like approval of maintainers first.

@sl0thentr0py
Copy link
Member

great suggestion @DanielNoord, thank you!
I agree that making the Transport contract explicit is helpful.

cc @antonpirker whenever we do this, we also need to clean up FunctionTransport since right now it only implements capture_event and is broken.

Also related to Custom Transports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants