feat(replay): support null filters on IPv4 #78642
Conversation
github-actions
bot
added
the
Scope: Backend
| "None", | ||
| "none", |
There was a problem hiding this comment.
I don't think these two should be special cases
Here are some examples of what the user can input, and what it looks like in the url/query param. Once it's in the url it becomes of type string. By special casing "None" and "none" we're treating those string values as equivalent to the string representation of pythons None value.
If we special case these then they should be in the docs, because, especially in context of replay, our users are not all python users familiar with this keyword. It's a put dealing with upper & lower case characters and all this stuff.
So I don't think we should special case these at all.
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #78642 +/- ##
===========================================
+ Coverage 67.19% 78.13% +10.93%
===========================================
Files 7097 7102 +5
Lines 312692 312775 +83
Branches 51069 51085 +16
===========================================
+ Hits 210117 244388 +34271
+ Misses 95651 62014 -33637
+ Partials 6924 6373 -551 |
| def visit_in(expression: Expression, value_list: list[str | None]) -> Condition: | ||
| values = [Function("toIPv4", parameters=[v]) for v in value_list if v is not None] | ||
| if None in value_list: | ||
| return Or( |
There was a problem hiding this comment.
Not sure if it's ok to return snuba_sdk.conditions.Or/And types here, instead of snuba_sdk.Condition. Or if we should maybe add a type annotation.
Passes typecheck, but my IDE is complaining. Maybe mypy is skipping stuff for snuba_sdk. @cmanallen do you have any thoughts?
Same applies to the visit_in and visit_not_in in aggregate.py
Closes #78591