ref: Typecheck test_consumer.py #3344
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the multistorage consumer completely ignores the policy defined on the storage itself and just applies it's own policy which produces the message to a topic. This is wrong and also different to how the main consumer works where the policy defined on the storage is the one that is used. It was also applied on a different part of the pipeline (only wraps the collector step). This also means the collector implementation was inconsistent in the two consumer variants. When this hack was first introduced, it's likely there were practical reasons we needed this at Sentry, but since this particular code path (multistorage consumer with dlq) is never used today, those obviously don't exist anymore. This is a step on the path to folding the multistorage consumer into the main consumer and making them behave in the same way.
The dlq part was the only one that didn't pass the type checks. Once that is removed in #3343 we no longer need to omit this file from typechecking.
untitaker
approved these changes
Nov 7, 2022
Codecov ReportBase: 92.36% // Head: 21.80% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3344 +/- ##
===========================================
- Coverage 92.36% 21.80% -70.56%
===========================================
Files 702 664 -38
Lines 32310 31119 -1191
===========================================
- Hits 29843 6787 -23056
- Misses 2467 24332 +21865
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The dlq part was the only bit that didn't pass the type checks in the consumer_test file. Once that is removed in #3343 we no longer need to omit this file from typechecking.