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
feat(envelope): Add some useful envelope methods #793
Conversation
ab976e5
to
6dbd284
Compare
23e7c08
to
2470fcd
Compare
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.
@dashed I see you want this for getsentry/relay#724, but since the code is not used neither here nor in the Relay PR, it is hard to judge whether it actually helps writing what you're trying to write ;)
One suggestion would be to add simple tests here in sentry-python to showcase the usage (I would not expect someone maintaining this repo to have to read Relay tests).
# t = ...
e = Envelope()
e.add_transaction(t)
with pytest.raises(ValueError):
e.add_transaction(t) # can only have 1 transaction in the envelope
assert e.get_transaction() is t
Since there can only be one transaction, perhaps a better API for the present would be a single property -- with the drawback of getting ugly if we ever allow more than 1 transaction per envelope. If that ever happens we can deprecate the property and add add_transaction
, perhaps not a big deal.
# t = ...
e = Envelope()
t.transaction = t # behind the scenes this either adds or replaces an envelope item
assert t.transaction == t
3113f0f
to
f0ba04e
Compare
f0ba04e
to
c41f33e
Compare
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Co-authored-by: Mark Story <mark@sentry.io>
7a06f7a
to
2dcba9d
Compare
@rhcarvalho I've added some tests 1e29b2a |
Add some useful envelope methods needed for integration tests in getsentry/relay#724
TODO