Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

StreamChangeCache.get_all_changed_entities incorrect usage #14583

Closed
4 of 6 tasks
clokep opened this issue Nov 29, 2022 · 3 comments · Fixed by #14592
Closed
4 of 6 tasks

StreamChangeCache.get_all_changed_entities incorrect usage #14583

clokep opened this issue Nov 29, 2022 · 3 comments · Fixed by #14592
Assignees
Labels
O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@clokep
Copy link
Member

clokep commented Nov 29, 2022

The get_all_changed_entities returns None in the case that the cache cannot provide information on the given stream ordering. Some of the callers treat None equivalent to [] (which corresponds to no changes).

Callers, checked do it correctly. Unchecked are either incorrect or maybe incorrect:

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience labels Nov 29, 2022
@clokep
Copy link
Member Author

clokep commented Nov 29, 2022

I think user impact would be missed presence updates and missed device change notifications?

@clokep
Copy link
Member Author

clokep commented Nov 29, 2022

Tangentially related to #14435.

@clokep
Copy link
Member Author

clokep commented Nov 29, 2022

I think in both these situations we need to pull the full list of user IDs from the database.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant