Skip to content

Commit

Permalink
Fix occasional AttributeError in `apps.grafana_plugin.tasks.sync.sy…
Browse files Browse the repository at this point in the history
…nc_organization_async` task (#3687)

# Which issue(s) this PR fixes

Fix this issue I came across in a celery task retry exception log:
![Screenshot 2024-01-15 at 11 21
13](https://github.com/grafana/oncall/assets/9406895/ed08f2f1-dc7d-4ad3-88a0-dc02cd740582)


## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
  • Loading branch information
joeyorlando authored and iskhakov committed Feb 20, 2024
1 parent b2d30fa commit e4bd9f4
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

## v1.3.87 (2024-01-15)

### Fixed

- Fix occasional `AttributeError` in `apps.grafana_plugin.tasks.sync.sync_organization_async` task by @joeyorlando ([#3687](https://github.com/grafana/oncall/pull/3687))

## v1.3.86 (2024-01-12)

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion engine/apps/grafana_plugin/helpers/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class GrafanaServiceAccountToken(typing.TypedDict):

class PluginSettings(typing.TypedDict):
enabled: bool
jsonData: typing.Dict[str, str]
jsonData: typing.NotRequired[typing.Dict[str, str]]

class TeamsResponse(_BaseGrafanaAPIResponse):
teams: typing.List["GrafanaAPIClient.Types.GrafanaTeam"]
Expand Down
2 changes: 1 addition & 1 deletion engine/apps/user_management/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def _sync_organization(organization: Organization) -> None:
grafana_incident_settings, _ = grafana_api_client.get_grafana_incident_plugin_settings()
if grafana_incident_settings is not None:
organization.is_grafana_incident_enabled = grafana_incident_settings["enabled"]
organization.grafana_incident_backend_url = grafana_incident_settings["jsonData"].get(
organization.grafana_incident_backend_url = grafana_incident_settings.get("jsonData", {}).get(
GrafanaAPIClient.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY
)
else:
Expand Down
24 changes: 17 additions & 7 deletions engine/apps/user_management/tests/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make


@pytest.mark.django_db
@pytest.mark.parametrize(
"get_grafana_incident_plugin_settings_return_value",
[
({"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, None),
# missing jsonData (sometimes this is what we get back from the Grafana API)
({"enabled": True}, None),
],
)
@patch.object(GrafanaAPIClient, "is_rbac_enabled_for_organization", return_value=False)
@patch.object(
GrafanaAPIClient,
Expand Down Expand Up @@ -212,21 +220,20 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make
),
)
@patch.object(GrafanaAPIClient, "check_token", return_value=(None, {"connected": True}))
@patch.object(
GrafanaAPIClient,
"get_grafana_incident_plugin_settings",
return_value=({"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, None),
)
@patch.object(GrafanaAPIClient, "get_grafana_incident_plugin_settings")
@patch("apps.user_management.sync.org_sync_signal")
def test_sync_organization(
mocked_org_sync_signal,
_mock_get_grafana_incident_plugin_settings,
mock_get_grafana_incident_plugin_settings,
_mock_check_token,
_mock_get_teams,
_mock_get_users,
_mock_is_rbac_enabled_for_organization,
get_grafana_incident_plugin_settings_return_value,
make_organization,
):
mock_get_grafana_incident_plugin_settings.return_value = get_grafana_incident_plugin_settings_return_value

organization = make_organization()

api_members_response = (
Expand Down Expand Up @@ -259,7 +266,10 @@ def test_sync_organization(

# check that is_grafana_incident_enabled flag is set
assert organization.is_grafana_incident_enabled is True
assert organization.grafana_incident_backend_url == MOCK_GRAFANA_INCIDENT_BACKEND_URL
if get_grafana_incident_plugin_settings_return_value[0].get("jsonData"):
assert organization.grafana_incident_backend_url == MOCK_GRAFANA_INCIDENT_BACKEND_URL
else:
assert organization.grafana_incident_backend_url is None

mocked_org_sync_signal.send.assert_called_once_with(sender=None, organization=organization)

Expand Down

0 comments on commit e4bd9f4

Please sign in to comment.