-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
instr(txnames): Count number of discovered rules #49085
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #49085 +/- ##
=======================================
Coverage 80.91% 80.92%
=======================================
Files 4819 4819
Lines 202002 202008 +6
Branches 11412 11412
=======================================
+ Hits 163459 163469 +10
+ Misses 38289 38285 -4
Partials 254 254
|
@@ -188,23 +190,33 @@ def get_sorted_rules(project: Project) -> List[Tuple[ReplacementRule, int]]: | |||
return ProjectOptionRuleStore().read_sorted(project) | |||
|
|||
|
|||
def update_rules(project: Project, new_rules: Sequence[ReplacementRule]) -> None: | |||
def update_rules(project: Project, new_rules: Sequence[ReplacementRule]) -> int: | |||
"""Write newly discovered rules to projection option and redis, and update last_used. |
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.
"""Write newly discovered rules to projection option and redis, and update last_used. | |
"""Writes newly discovered rules to project option and Redis, and updates last_used. |
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.
Python has different conventions than Rust:
The docstring is a phrase ending in a period. It prescribes the function or method’s effect as a command (“Do this”, “Return that”), not as a description; e.g. don’t write “Returns the pathname …”.
https://peps.python.org/pep-0257/#one-line-docstrings
I'll fix the "Returns" below.
@@ -188,23 +190,33 @@ def get_sorted_rules(project: Project) -> List[Tuple[ReplacementRule, int]]: | |||
return ProjectOptionRuleStore().read_sorted(project) | |||
|
|||
|
|||
def update_rules(project: Project, new_rules: Sequence[ReplacementRule]) -> None: | |||
def update_rules(project: Project, new_rules: Sequence[ReplacementRule]) -> int: | |||
"""Write newly discovered rules to projection option and redis, and update last_used. |
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.
Python has different conventions than Rust:
The docstring is a phrase ending in a period. It prescribes the function or method’s effect as a command (“Do this”, “Return that”), not as a description; e.g. don’t write “Returns the pathname …”.
https://peps.python.org/pep-0257/#one-line-docstrings
I'll fix the "Returns" below.
In order to detect regressions in our transaction clusterer, keep track of the number of new rules discovered across projects.
ref: https://github.com/getsentry/team-ingest/issues/124