Skip to content
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

bug-1901998: fix buginfo view to stop sending sentry errors #6779

Merged
merged 1 commit into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions webapp/crashstats/crashstats/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def get_bugs_and_related_bugs(self, signatures):


class BugAssociation(models.Model):
"""Specifies assocations between bug ids in Bugzilla and signatures"""
"""Specifies associations between bug ids in Bugzilla and signatures"""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set up pre-commit and it's now fixing typos before I can create commits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that work for all local development, or is that config specific to your local development? If the former, where was that added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used it a long time ago and then decided I didn't like the way it was structured. Rehan built Therapist which I used for a while, but I think that's unmaintained now.

I noticed SRE uses pre-commit for their repositories and decided to try it out again.

https://github.com/mozilla-it/webservices-infra/blob/main/.pre-commit-config.yaml
https://github.com/mozilla/terraform-modules/blob/main/.pre-commit-config.yaml
https://github.com/mozilla-it/global-platform-admin/blob/main/.pre-commit-config.yaml

I was going to give it a little more time and then bring it up at an Observeneers meeting.

In the meantime, it's fixing spelling in PRs I'm doing in files I made a change to and I wanted to point out that's a cosmetic change.


bug_id = models.IntegerField(null=False, help_text="Bugzilla bug id")
signature = models.TextField(
Expand Down Expand Up @@ -244,7 +244,7 @@ class ParameterTypeError(Exception):


def _clean_path(path):
"""Santize a URL path
"""Sanitize a URL path

:arg path: the path to sanitize

Expand Down Expand Up @@ -608,7 +608,7 @@ class ProcessedCrash(SocorroMiddleware):
HELP_TEXT = """
API for retrieving crash data generated by processing.

Requires protected data acess to view protected parts of the processed crash.
Requires protected data access to view protected parts of the processed crash.

Params:

Expand Down Expand Up @@ -1027,6 +1027,9 @@ def get(self, bugs, **kwargs):
# 503 = Service Unavailable
# 504 = Gateway Time-out
status_forcelist=(500, 502, 503, 504),
# This changes backoff to (0.2, 0.4, 0.8, 1.6, 3.2) which
# gives Bugzilla a better chance of responding helpfully
backoff_factor=0.2,
)
response = session.get(url, headers=headers)
if response.status_code != 200:
Expand Down
17 changes: 17 additions & 0 deletions webapp/crashstats/crashstats/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import requests_mock
import pyquery
from markus.testing import MetricsMock
from requests.exceptions import RetryError

from django.conf import settings
from django.contrib.auth import REDIRECT_FIELD_NAME
Expand Down Expand Up @@ -358,6 +359,22 @@ def mocked_get(url, **options):
cache_key = "buginfo:987"
assert cache.get(cache_key) == struct["bugs"][0]

def test_buginfo_retry_error(self, client):
with requests_mock.Mocker(real_http=False) as mock_requests:
mock_requests.get(
(
"http://bugzilla.example.com/rest/bug?"
+ "id=1901998&include_fields=summary,status,id,resolution"
),
exc=RetryError,
)

url = reverse("crashstats:buginfo")
response = client.get(url, {"bug_ids": "1901998"})
assert response.status_code == 500
json_response = json.loads(response.content)
assert json_response == {"error": "Max retries exceeded with Bugzilla."}


class Test_quick_search:
def test_quick_search(self, client):
Expand Down
7 changes: 5 additions & 2 deletions webapp/crashstats/crashstats/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from urllib.parse import quote

import glom
from requests.exceptions import RetryError

from django import http
from django.conf import settings
Expand Down Expand Up @@ -332,8 +333,10 @@ def buginfo(request, signatures=None):
bug_ids = form.cleaned_data["bug_ids"]

bzapi = models.BugzillaBugInfo()
result = bzapi.get(bug_ids)
return result
try:
return bzapi.get(bug_ids)
except RetryError:
return {"error": "Max retries exceeded with Bugzilla."}, 500
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return is a structure that utils.json_view recognizes and will split into "response, status".



@pass_default_context
Expand Down