-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
ref(rules): Update typing in rule files #69068
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -236,8 +245,9 @@ def apply_rule(self, rule: Rule, status: GroupRuleStatus) -> None: | |||
if not predicate_func(predicate_iter): | |||
return | |||
else: | |||
self.logger.error( | |||
f"Unsupported {name}_match {match!r} for rule {rule.id}", | |||
log_string = f"Unsupported {name}_match {match!r} for rule {rule.id}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to be in a separate var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got some error if i didn't: LOG011 avoid pre-formatting log messages
@@ -319,7 +346,7 @@ class EventFrequencyCondition(BaseEventFrequencyCondition): | |||
label = "The issue is seen more than {value} times in {interval}" | |||
|
|||
def query_hook( | |||
self, event: GroupEvent, start: datetime, end: datetime, environment_id: str | |||
self, event: GroupEvent, start: datetime, end: datetime, environment_id: int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooo is it the ID and not the text now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was just wrong before
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #69068 +/- ##
===========================================
+ Coverage 58.20% 79.64% +21.43%
===========================================
Files 6420 6429 +9
Lines 284855 285226 +371
Branches 49078 49128 +50
===========================================
+ Hits 165809 227161 +61352
+ Misses 118650 57674 -60976
+ Partials 396 391 -5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i notice there is some test coverage lacking on these changes -- worth adding some now?
Pulls out the non-functional changes from #68517 which are mostly typing related.