Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test(ingest): add DLQ tests for txns and attachments and DLQ support for attachments #67495

Merged
merged 5 commits into from Mar 25, 2024
Merged

test(ingest): add DLQ tests for txns and attachments and DLQ support for attachments #67495

merged 5 commits into from Mar 25, 2024

Conversation

aliu3ntry
Copy link
Member

No description provided.

@aliu3ntry aliu3ntry requested a review from a team as a code owner March 21, 2024 23:46
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 21, 2024
@codecov Codecov
Copy link

codecov bot commented Mar 21, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.36%. Comparing base (8455135) to head (bedfad5).

❗ Current head bedfad5 differs from pull request most recent head 68f0b3a. Consider uploading reports for the commit 68f0b3a to get more accurate results

Additional details and impacted files 
@@             Coverage Diff             @@
##           master   #67495       +/-   ##
===========================================
+ Coverage   47.37%   79.36%   +31.98%     
===========================================
  Files        1840     6370     +4530     
  Lines      118052   282227   +164175     
  Branches    21321    48564    +27243     
===========================================
+ Hits        55932   223992   +168060     
+ Misses      62120    57795     -4325     
- Partials        0      440      +440
Files Coverage Δ
src/sentry/ingest/consumer/attachment_event.py 90.69% <100.00%> (ø)

... and 5760 files with indirect coverage changes

@aliu3ntry aliu3ntry changed the title test(ingest): add testing for transaction and attachment DLQs test(ingest): add DLQ tests for txns and attachments and DLQ support for attach.s Mar 21, 2024
@aliu3ntry aliu3ntry changed the title test(ingest): add DLQ tests for txns and attachments and DLQ support for attach.s test(ingest): add DLQ tests for txns and attachments and DLQ support for attachments Mar 21, 2024
@cmanallen
Copy link
Member

Looks good to me. I'll let @lynnagara determine if the DLQ implementation is correct.

lynnagara
lynnagara approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@lynnagara lynnagara left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@aliu3ntry aliu3ntry merged commit 497d057 into master Mar 25, 2024
48 checks passed
@aliu3ntry aliu3ntry deleted the aliu/dlq-tests branch March 25, 2024 17:38
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jjbayer jjbayer jjbayer approved these changes

@lynnagara lynnagara lynnagara approved these changes

@cmanallen cmanallen Awaiting requested review from cmanallen

@JoshFerge JoshFerge Awaiting requested review from JoshFerge

Assignees
No one assigned
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aliu3ntry @cmanallen @jjbayer @lynnagara