Skip to content

Conversation

@mgaligniana
Copy link
Contributor

Fixes GH-2696

@mgaligniana mgaligniana marked this pull request as draft December 22, 2024 03:28
@codecov
Copy link

codecov bot commented Dec 22, 2024

Codecov Report

Attention: Patch coverage is 87.80488% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.55%. Comparing base (eeedd11) to head (89160fc).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/fastapi.py 50.00% 1 Missing ⚠️
sentry_sdk/integrations/gcp.py 0.00% 1 Missing ⚠️
sentry_sdk/integrations/litestar.py 50.00% 1 Missing ⚠️
sentry_sdk/integrations/starlette.py 66.66% 1 Missing ⚠️
sentry_sdk/integrations/starlite.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3889      +/-   ##
==========================================
+ Coverage   79.53%   79.55%   +0.02%     
==========================================
  Files         140      140              
  Lines       15517    15521       +4     
  Branches     2630     2631       +1     
==========================================
+ Hits        12341    12348       +7     
+ Misses       2340     2338       -2     
+ Partials      836      835       -1     
Files with missing lines Coverage Δ
sentry_sdk/integrations/aiohttp.py 81.25% <ø> (ø)
sentry_sdk/integrations/arq.py 89.84% <100.00%> (ø)
sentry_sdk/integrations/asgi.py 85.61% <100.00%> (ø)
sentry_sdk/integrations/aws_lambda.py 30.91% <100.00%> (ø)
sentry_sdk/integrations/celery/__init__.py 86.55% <100.00%> (ø)
sentry_sdk/integrations/chalice.py 89.18% <100.00%> (ø)
sentry_sdk/integrations/django/__init__.py 81.48% <100.00%> (ø)
sentry_sdk/integrations/grpc/aio/server.py 86.44% <100.00%> (ø)
sentry_sdk/integrations/grpc/server.py 76.31% <100.00%> (ø)
sentry_sdk/integrations/huey.py 16.66% <ø> (ø)
... and 13 more

... and 3 files with indirect coverage changes

@mgaligniana mgaligniana force-pushed the GH-2696-transaction-source-enum branch 2 times, most recently from f4bb234 to ef97faa Compare December 22, 2024 04:51
@mgaligniana
Copy link
Contributor Author

Linter is failing because of:

image

Should I fix them in a different commit?

@mgaligniana mgaligniana marked this pull request as ready for review December 22, 2024 04:54
Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thanks for the cleanup

@antonpirker
Copy link
Contributor

antonpirker commented Dec 23, 2024

Linter is failing because of:

image

Should I fix them in a different commit?

no, this is because of a new release of mypy. I will fix this.

@antonpirker antonpirker enabled auto-merge (squash) December 23, 2024 09:02
@antonpirker
Copy link
Contributor

This breaks a lot of tests, because the full enum ends up in the envelope that is sent to Sentry and not the .value of the enum. Can you please fix this?

Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the tests, by making sure the .value of the enum (the string) ends up in the envelop json payload that is sent to Sentry.

auto-merge was automatically disabled December 23, 2024 13:37

Head branch was pushed to by a user without write access

@mgaligniana mgaligniana force-pushed the GH-2696-transaction-source-enum branch 4 times, most recently from b016caf to 79f20d7 Compare December 23, 2024 13:51
@mgaligniana
Copy link
Contributor Author

Thank you @antonpirker! Now linter and tests passing! 💪

@mgaligniana mgaligniana force-pushed the GH-2696-transaction-source-enum branch from 79f20d7 to e135bbc Compare January 14, 2025 13:26
@mgaligniana mgaligniana force-pushed the GH-2696-transaction-source-enum branch from e135bbc to 6609d89 Compare January 15, 2025 12:01
@mgaligniana
Copy link
Contributor Author

Hi Anton! Just curious why did we have to delete the .value again. Let me know if there is something I can help

@antonpirker
Copy link
Contributor

I just made it so one does not have to always use the .value, so the enum is easier to use and error due to missing .value are not happening. otherwise it is a really cool change and I will merge it today

Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now ready to be merged!

@antonpirker antonpirker merged commit 189e4a9 into getsentry:master Feb 24, 2025
143 of 145 checks passed
@mgaligniana
Copy link
Contributor Author

Thank you @antonpirker !!

Comment on lines 142 to 143
def __str__(self):
return self.value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see this! Thanks for the explanation!

scottbarnes added a commit to scottbarnes/openlibrary that referenced this pull request Aug 26, 2025
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

Successfully merging this pull request may close these issues.

Transaction Source Enum

2 participants