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.

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

Introduce a new stream for the appservice consumer #3277

Merged
merged 8 commits into from
Dec 12, 2023
Merged

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Dec 1, 2023

This introduces a new stream the syncAPI produces to once it processed a OutputRoomEvent and the appservices consumes.
This is to work around a race condition where appservices receive an event before the syncAPI has handled it, this can result in e.g. calls to /joined_members returning a wrong membership list.

@S7evinK S7evinK added C-Sync-API C-App-Services C-NATS T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Dec 1, 2023
@S7evinK S7evinK requested a review from a team as a code owner December 1, 2023 07:30
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (fd11e65) 65.82% compared to head (c08df16) 65.42%.
Report is 1 commits behind head on main.

Files Patch % Lines
appservice/consumers/roomserver.go 62.50% 2 Missing and 1 partial ⚠️
syncapi/consumers/roomserver.go 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3277      +/-   ##
==========================================
- Coverage   65.82%   65.42%   -0.40%     
==========================================
  Files         508      509       +1     
  Lines       57392    57566     +174     
==========================================
- Hits        37776    37663     -113     
- Misses      15803    16046     +243     
- Partials     3813     3857      +44     
Flag Coverage Δ
unittests 49.75% <75.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@S7evinK S7evinK merged commit 1555b35 into main Dec 12, 2023
20 checks passed
@S7evinK S7evinK deleted the s7evink/as-producer branch December 12, 2023 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-App-Services C-NATS C-Sync-API T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant