Skip to content

Commit

Permalink
chore(various): Fix linter warnings (#82494)
Browse files Browse the repository at this point in the history
This fixes a number of small issues my linter has been yelling at me about, which I've come across in the last handful of weeks - things like unused variables, shadowing built-ins, etc. No behavior changes just tiny refactors to clear out some noise.
  • Loading branch information
lobsterkatie authored and andrewshie-sentry committed Jan 2, 2025
1 parent e999f0f commit 8a4ea04
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 79 deletions.
4 changes: 2 additions & 2 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2376,15 +2376,15 @@ def save_attachment(
return
from sentry import ratelimits as ratelimiter

is_limited, num_requests, reset_time = ratelimiter.backend.is_limited_with_value(
is_limited, _, _ = ratelimiter.backend.is_limited_with_value(
key="event_attachment.save_per_sec",
limit=options.get("sentry.save-event-attachments.project-per-sec-limit"),
project=project,
window=1,
)
rate_limit_tag = "per_sec"
if not is_limited:
is_limited, num_requests, reset_time = ratelimiter.backend.is_limited_with_value(
is_limited, _, _ = ratelimiter.backend.is_limited_with_value(
key="event_attachment.save_5_min",
limit=options.get("sentry.save-event-attachments.project-per-5-minute-limit"),
project=project,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/grouping/enhancer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def loads(cls, data) -> Enhancements:

@classmethod
@sentry_sdk.tracing.trace
def from_config_string(self, s, bases=None, id=None) -> Enhancements:
def from_config_string(cls, s, bases=None, id=None) -> Enhancements:
rust_enhancements = parse_rust_enhancements("config_string", s)

rules = parse_enhancements(s)
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/grouping/enhancer/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def is_updater(self) -> bool:
def _from_config_structure(cls, val, version: int):
if isinstance(val, list):
return VarAction(val[0], val[1])
flag, range = REVERSE_ACTION_FLAGS[val >> ACTION_BITSIZE]
return FlagAction(ACTIONS[val & 0xF], flag, range)
flag, range_direction = REVERSE_ACTION_FLAGS[val >> ACTION_BITSIZE]
return FlagAction(ACTIONS[val & 0xF], flag, range_direction)


class FlagAction(EnhancementAction):
Expand Down
13 changes: 3 additions & 10 deletions src/sentry/grouping/fingerprinting/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,20 +452,13 @@ def _positive_match(self, values: dict[str, Any]) -> bool:
value = values.get(self.key)
if value is None:
return False
elif self.key == "package":
elif self.key in ["package", "release"]:
if self._positive_path_match(value):
return True
elif self.key == "family":
elif self.key in ["family", "sdk"]:
flags = self.pattern.split(",")
if "all" in flags or value in flags:
return True
elif self.key == "sdk":
flags = self.pattern.split(",")
if "all" in flags or value in flags:
return True
elif self.key == "release":
if self._positive_path_match(value):
return True
elif self.key == "app":
ref_val = bool_from_string(self.pattern)
if ref_val is not None and ref_val == value:
Expand Down Expand Up @@ -591,7 +584,7 @@ def visit_fingerprinting_rules(
in_header = True
for child in children:
if isinstance(child, str):
if in_header and child[:2] == "##":
if in_header and child.startswith("##"):
changelog.append(child[2:].rstrip())
else:
in_header = False
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/grouping/strategies/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ def is_recursion_legacy(frame1: Frame, frame2: Frame) -> bool:
def remove_module_outliers_legacy(module: str, platform: str) -> tuple[str, str | None]:
"""Remove things that augment the module but really should not."""
if platform == "java":
if module[:35] == "sun.reflect.GeneratedMethodAccessor":
if module.startswith("sun.reflect.GeneratedMethodAccessor"):
return "sun.reflect.GeneratedMethodAccessor", "removed reflection marker"
if module[:44] == "jdk.internal.reflect.GeneratedMethodAccessor":
if module.startswith("jdk.internal.reflect.GeneratedMethodAccessor"):
return "jdk.internal.reflect.GeneratedMethodAccessor", "removed reflection marker"
old_module = module
module = _java_reflect_enhancer_re.sub(r"\1<auto>", module)
Expand Down
5 changes: 2 additions & 3 deletions src/sentry/grouping/strategies/newstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ def get_filename_component(
new_filename = _java_assist_enhancer_re.sub(r"\1<auto>", filename)
if new_filename != filename:
filename_component.update(values=[new_filename], hint="cleaned javassist parts")
filename = new_filename

return filename_component

Expand All @@ -176,11 +175,11 @@ def get_module_component(
elif platform == "java":
if "$$Lambda$" in module:
module_component.update(contributes=False, hint="ignored java lambda")
if module[:35] == "sun.reflect.GeneratedMethodAccessor":
if module.startswith("sun.reflect.GeneratedMethodAccessor"):
module_component.update(
values=["sun.reflect.GeneratedMethodAccessor"], hint="removed reflection marker"
)
elif module[:44] == "jdk.internal.reflect.GeneratedMethodAccessor":
elif module.startswith("jdk.internal.reflect.GeneratedMethodAccessor"):
module_component.update(
values=["jdk.internal.reflect.GeneratedMethodAccessor"],
hint="removed reflection marker",
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/utils/tag_normalization.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def normalize_sdk_tag(tag: str) -> str:

# collapse tags other than JavaScript / Native to their top-level SDK

if not tag.split(".")[1] in {"javascript", "native"}:
if tag.split(".")[1] not in {"javascript", "native"}:
tag = ".".join(tag.split(".", 2)[0:2])

if tag.split(".")[1] == "native":
Expand Down
6 changes: 2 additions & 4 deletions tests/sentry/event_manager/test_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ def test_group_release_no_env(self) -> None:
).exists()

# ensure we're not erroring on second creation
event = self.make_release_event("1.0", project_id)
self.make_release_event("1.0", project_id)

def test_group_release_with_env(self) -> None:
manager = EventManager(make_event(release="1.0", environment="prod", event_id="a" * 32))
Expand Down Expand Up @@ -1891,9 +1891,7 @@ def test_throws_when_matches_discarded_hash(self) -> None:
with self.feature("organizations:event-attachments"):
with self.tasks():
with pytest.raises(HashDiscarded):
event = manager.save(
self.project.id, cache_key=cache_key, has_attachments=True
)
manager.save(self.project.id, cache_key=cache_key, has_attachments=True)

assert mock_track_outcome.call_count == 3

Expand Down
76 changes: 38 additions & 38 deletions tests/sentry/grouping/ingest/test_seer.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,19 @@ def setUp(self) -> None:

@patch("sentry.grouping.ingest.seer.get_similarity_data_from_seer", return_value=[])
def test_sends_expected_data_to_seer(self, mock_get_similarity_data: MagicMock) -> None:
type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="12312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand All @@ -275,7 +275,7 @@ def test_sends_expected_data_to_seer(self, mock_get_similarity_data: MagicMock)
"event_id": new_event.event_id,
"hash": new_event.get_primary_hash(),
"project_id": self.project.id,
"stacktrace": f'{type}: {value}\n File "dogpark.py", function play_fetch\n {context_line}',
"stacktrace": f'{error_type}: {error_value}\n File "dogpark.py", function play_fetch\n {context_line}',
"exception_type": "FailedToFetchError",
"k": 1,
"referrer": "ingest",
Expand Down Expand Up @@ -353,19 +353,19 @@ def test_returns_no_grouphash_and_empty_metadata_if_empty_stacktrace(
def test_too_many_frames(
self, mock_metrics: Mock, mock_record_did_call_seer: MagicMock
) -> None:
type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="22312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down Expand Up @@ -401,19 +401,19 @@ def test_too_many_frames(

@patch("sentry.seer.similarity.utils.record_did_call_seer_metric")
def test_too_many_frames_allowed_platform(self, mock_record_did_call_seer: MagicMock) -> None:
type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="22312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down Expand Up @@ -453,19 +453,19 @@ def test_valid_maybe_check_seer_for_matching_group_hash(
) -> None:
self.project.update_option("sentry:similarity_backfill_completed", int(time()))

type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="12312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down Expand Up @@ -494,7 +494,7 @@ def test_valid_maybe_check_seer_for_matching_group_hash(
"event_id": new_event.event_id,
"hash": new_event.get_primary_hash(),
"project_id": self.project.id,
"stacktrace": f'{type}: {value}\n File "dogpark.py", function play_fetch\n {context_line}',
"stacktrace": f'{error_type}: {error_value}\n File "dogpark.py", function play_fetch\n {context_line}',
"exception_type": "FailedToFetchError",
"k": 1,
"referrer": "ingest",
Expand All @@ -513,19 +513,19 @@ def test_too_many_frames_maybe_check_seer_for_matching_group_hash(
) -> None:
self.project.update_option("sentry:similarity_backfill_completed", int(time()))

type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="22312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down Expand Up @@ -572,19 +572,19 @@ def test_too_many_frames_maybe_check_seer_for_matching_group_hash_bypassed_platf
) -> None:
self.project.update_option("sentry:similarity_backfill_completed", int(time()))

type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
new_event = Event(
project_id=self.project.id,
event_id="22312012112120120908201304152013",
data={
"title": f"{type}('{value}')",
"title": f"{error_type}('{error_value}')",
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down
10 changes: 5 additions & 5 deletions tests/sentry/grouping/test_parameterization.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ def test_parameterize_regex_experiment():
regex_pattern_keys=(),
experiments=(FooExperiment,),
)
input = "blah foobarbaz fooooo"
normalized = parameterizer.parameterize_all(input)
input_str = "blah foobarbaz fooooo"
normalized = parameterizer.parameterize_all(input_str)
assert normalized == "blah <foo>barbaz <foo>ooo"
assert len(parameterizer.get_successful_experiments()) == 1
assert parameterizer.get_successful_experiments()[0] == FooExperiment
Expand All @@ -261,9 +261,9 @@ def test_parameterize_regex_experiment_cached_compiled():
regex_pattern_keys=(),
experiments=(FooExperiment,),
)
input = "blah foobarbaz fooooo"
_ = parameterizer.parameterize_all(input)
_ = parameterizer.parameterize_all(input)
input_str = "blah foobarbaz fooooo"
_ = parameterizer.parameterize_all(input_str)
_ = parameterizer.parameterize_all(input_str)

mocked_pattern.assert_called_once()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,15 +686,15 @@ def test_obeys_useReranking_query_param(self, mock_seer_request: mock.MagicMock)
mock_seer_request.reset_mock()

def test_too_many_system_frames(self) -> None:
type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
error_data = {
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand All @@ -719,15 +719,15 @@ def test_too_many_system_frames(self) -> None:
assert response.data == []

def test_no_filename_or_module(self) -> None:
type = "FailedToFetchError"
value = "Charlie didn't bring the ball back"
context_line = f"raise {type}('{value}')"
error_type = "FailedToFetchError"
error_value = "Charlie didn't bring the ball back"
context_line = f"raise {error_type}('{error_value}')"
error_data = {
"exception": {
"values": [
{
"type": type,
"value": value,
"type": error_type,
"value": error_value,
"stacktrace": {
"frames": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def test_does_not_create_broken_detection_insufficient_duration(self):
grouphash=hash_from_values([uuid.uuid4()]),
)

for i in range(4):
for _ in range(4):
MonitorCheckIn.objects.create(
monitor=monitor,
monitor_environment=monitor_environment,
Expand Down

0 comments on commit 8a4ea04

Please sign in to comment.