From 0b0ecef72ce46e321da56adc7f89fadbeba23374 Mon Sep 17 00:00:00 2001 From: Dominik Broj Date: Fri, 1 Dec 2023 10:38:59 +0100 Subject: [PATCH 1/6] Pass correct params when using retryFailingPromises (#3480) # What this PR does Pass correct params to functions when using retryFailingPromises ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/3479 ## 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) --- grafana-plugin/src/state/rootBaseStore/index.ts | 8 ++++---- grafana-plugin/src/utils/async.test.ts | 12 +++++++++--- grafana-plugin/src/utils/async.ts | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index 76d20346..6acd40ed 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -125,10 +125,10 @@ export class RootBaseStore { }; await retryFailingPromises([ - this.userStore.loadCurrentUser, - this.organizationStore.loadCurrentOrganization, - this.grafanaTeamStore.updateItems, - updateFeatures, + () => this.userStore.loadCurrentUser(), + () => this.organizationStore.loadCurrentOrganization(), + () => this.grafanaTeamStore.updateItems(), + () => updateFeatures(), ]); this.isBasicDataLoaded = true; } diff --git a/grafana-plugin/src/utils/async.test.ts b/grafana-plugin/src/utils/async.test.ts index 5c943f66..289ecfc6 100644 --- a/grafana-plugin/src/utils/async.test.ts +++ b/grafana-plugin/src/utils/async.test.ts @@ -8,7 +8,10 @@ describe('retryFailingPromises', () => { let attempts1 = 0; let attempts2 = 0; let attempts3 = 0; - const fetch1 = async () => Promise.resolve(++attempts1); + const fetch1 = (param = 'param') => { + ++attempts1; + return Promise.resolve(param); + }; const fetch2 = async () => Promise.reject(++attempts2); const fetch3 = async () => new Promise((resolve, reject) => { @@ -19,13 +22,16 @@ describe('retryFailingPromises', () => { reject(attempts3); }); - const result = await retryFailingPromises([fetch1, fetch2, fetch3], { maxAttempts: MAX_ATTEMPTS, delayInMs: 50 }); + const result = await retryFailingPromises([() => fetch1(), fetch2, fetch3], { + maxAttempts: MAX_ATTEMPTS, + delayInMs: 50, + }); expect(attempts1).toBe(1); expect(attempts2).toBe(MAX_ATTEMPTS); expect(attempts3).toBe(2); expect(result).toEqual([ - { status: 'fulfilled', value: 1 }, + { status: 'fulfilled', value: 'param' }, { status: 'rejected', reason: 5 }, { status: 'fulfilled', value: 2 }, ]); diff --git a/grafana-plugin/src/utils/async.ts b/grafana-plugin/src/utils/async.ts index 0ebafde1..ad6a3c10 100644 --- a/grafana-plugin/src/utils/async.ts +++ b/grafana-plugin/src/utils/async.ts @@ -1,7 +1,7 @@ -import { retry } from '@lifeomic/attempt'; +import { AttemptContext, retry } from '@lifeomic/attempt'; export const retryFailingPromises = async ( - asyncActions: Array<() => Promise>, + asyncActions: Array<(ctx?: AttemptContext) => Promise>, { maxAttempts = 3, delayInMs = 500 }: { maxAttempts?: number; delayInMs?: number } = {} ) => maxAttempts === 0 From a56d39b91760af1c96273765fa7f0866d82246da Mon Sep 17 00:00:00 2001 From: Dominik Broj Date: Fri, 1 Dec 2023 11:00:49 +0100 Subject: [PATCH 2/6] Fix monaco editor height (#3467) # What this PR does Use css height: 100% instead of manually calculating height of monaco editor ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/2351 ## 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) --- grafana-plugin/src/assets/style/utils.css | 5 +++++ .../components/MonacoEditor/MonacoEditor.tsx | 2 +- .../IntegrationTemplate.module.scss | 4 ++++ .../IntegrationTemplate.tsx | 14 ++----------- .../TemplatesAlertGroupsList.module.css | 8 ++++++++ .../TemplatesAlertGroupsList.tsx | 20 +++---------------- .../WebhooksTemplateEditor.tsx | 16 +++------------ 7 files changed, 26 insertions(+), 43 deletions(-) diff --git a/grafana-plugin/src/assets/style/utils.css b/grafana-plugin/src/assets/style/utils.css index 7b02d032..b562bbf1 100644 --- a/grafana-plugin/src/assets/style/utils.css +++ b/grafana-plugin/src/assets/style/utils.css @@ -64,6 +64,11 @@ height: 100%; } +.u-width-height-100 { + width: 100%; + height: 100%; +} + .u-display-block { display: block; } diff --git a/grafana-plugin/src/components/MonacoEditor/MonacoEditor.tsx b/grafana-plugin/src/components/MonacoEditor/MonacoEditor.tsx index bcfbcb9b..5ce5619b 100644 --- a/grafana-plugin/src/components/MonacoEditor/MonacoEditor.tsx +++ b/grafana-plugin/src/components/MonacoEditor/MonacoEditor.tsx @@ -100,7 +100,7 @@ const MonacoEditor: FC = (props) => { height={height} onEditorDidMount={handleMount} getSuggestions={useAutoCompleteList ? autoCompleteList : undefined} - containerStyles="u-width-100" + containerStyles="u-width-height-100" /> ); }; diff --git a/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.module.scss b/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.module.scss index 1cc76968..8bcab258 100644 --- a/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.module.scss +++ b/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.module.scss @@ -65,3 +65,7 @@ .template-block-codeeditor div[aria-label='Code editor container'] { border-bottom: none; } + +.template-editor-block-content { + height: calc(100% - 60px); +} diff --git a/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.tsx b/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.tsx index 98e98b71..c747be2a 100644 --- a/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.tsx +++ b/grafana-plugin/src/containers/IntegrationTemplate/IntegrationTemplate.tsx @@ -23,7 +23,6 @@ import { AlertTemplatesDTO } from 'models/alert_templates'; import { Alert } from 'models/alertgroup/alertgroup.types'; import { ChannelFilter } from 'models/channel_filter/channel_filter.types'; import { TemplateOptions } from 'pages/integration/Integration.config'; -import { waitForElement } from 'utils/DOM'; import LocationHelper from 'utils/LocationHelper'; import { UserActions } from 'utils/authorization'; @@ -50,7 +49,6 @@ const IntegrationTemplate = observer((props: IntegrationTemplateProps) => { const [alertGroupPayload, setAlertGroupPayload] = useState(undefined); const [changedTemplateBody, setChangedTemplateBody] = useState(templateBody); const [resultError, setResultError] = useState(undefined); - const [editorHeight, setEditorHeight] = useState(undefined); const [isRecentAlertGroupExisting, setIsRecentAlertGroupExisting] = useState(false); useEffect(() => { @@ -63,14 +61,6 @@ const IntegrationTemplate = observer((props: IntegrationTemplateProps) => { } }, []); - useEffect(() => { - waitForElement('#content-container-id').then(() => { - const mainDiv = document.getElementById('content-container-id'); - const height = mainDiv?.getBoundingClientRect().height - 59; - setEditorHeight(`${height}px`); - }); - }, []); - const onShowCheatSheet = useCallback(() => { setIsCheatSheetVisible(true); }, []); @@ -188,7 +178,7 @@ const IntegrationTemplate = observer((props: IntegrationTemplateProps) => { width={'95%'} >
-
+
{ value={changedTemplateBody} data={templates} showLineNumbers={true} - height={editorHeight} + height="100%" onChange={getChangeHandler()} />
diff --git a/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.module.css b/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.module.css index f77bdc16..dc42a0f0 100644 --- a/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.module.css +++ b/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.module.css @@ -40,11 +40,19 @@ background-color: var(--background-secondary); } +.alert-groups-editor { + height: calc(100% - 60px); +} + .alert-groups-editor div[aria-label='Code editor container'] { border-bottom: none; border-right: none; } +.alert-groups-editor-withBadge { + height: calc(100% - 60px); +} + .alert-groups-editor-withBadge div[aria-label='Code editor container'] { background-color: var(--box-background); border-bottom: none; diff --git a/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.tsx b/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.tsx index 33102a8f..9f15792c 100644 --- a/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.tsx +++ b/grafana-plugin/src/containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList.tsx @@ -17,8 +17,6 @@ import { useStore } from 'state/useStore'; import styles from './TemplatesAlertGroupsList.module.css'; const cx = cn.bind(styles); -const HEADER_OF_CONTAINER_HEIGHT = 59; -const BADGE_WITH_PADDINGS_HEIGHT = 42; export enum TEMPLATE_PAGE { Integrations, @@ -71,18 +69,6 @@ const TemplatesAlertGroupsList = (props: TemplatesAlertGroupsListProps) => { } }, []); - const getCodeEditorHeight = () => { - const mainDiv = document.getElementById('alerts-content-container-id'); - const height = mainDiv?.getBoundingClientRect().height - HEADER_OF_CONTAINER_HEIGHT; - return `${height}px`; - }; - - const getCodeEditorHeightWithBadge = () => { - const mainDiv = document.getElementById('alerts-content-container-id'); - const height = mainDiv?.getBoundingClientRect().height - HEADER_OF_CONTAINER_HEIGHT - BADGE_WITH_PADDINGS_HEIGHT; - return `${height}px`; - }; - const getChangeHandler = () => { return debounce((value: string) => { onEditPayload(value); @@ -158,7 +144,7 @@ const TemplatesAlertGroupsList = (props: TemplatesAlertGroupsListProps) => { ...MONACO_EDITABLE_CONFIG, readOnly: false, }} - height={getCodeEditorHeight()} + height="100%" onChange={getChangeHandler()} />
@@ -271,7 +257,7 @@ const TemplatesAlertGroupsList = (props: TemplatesAlertGroupsListProps) => { { value={JSON.stringify(selectedPayload, null, 4)} data={undefined} disabled - height={getCodeEditorHeightWithBadge()} + height="100%" onChange={getChangeHandler()} useAutoCompleteList={false} language={MONACO_LANGUAGE.json} diff --git a/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx b/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx index aeb559df..093de6db 100644 --- a/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx +++ b/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { Button, Drawer, HorizontalGroup, VerticalGroup } from '@grafana/ui'; import cn from 'classnames/bind'; @@ -13,7 +13,6 @@ import TemplateResult from 'containers/TemplateResult/TemplateResult'; import TemplatesAlertGroupsList, { TEMPLATE_PAGE } from 'containers/TemplatesAlertGroupsList/TemplatesAlertGroupsList'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; import { OutgoingWebhook } from 'models/outgoing_webhook/outgoing_webhook.types'; -import { waitForElement } from 'utils/DOM'; import { UserActions } from 'utils/authorization'; const cx = cn.bind(styles); @@ -35,18 +34,9 @@ interface WebhooksTemplateEditorProps { const WebhooksTemplateEditor: React.FC = ({ template, id, onHide, handleSubmit }) => { const [isCheatSheetVisible, setIsCheatSheetVisible] = useState(false); const [changedTemplateBody, setChangedTemplateBody] = useState(template.value); - const [editorHeight, setEditorHeight] = useState(undefined); const [selectedPayload, setSelectedPayload] = useState(undefined); const [resultError, setResultError] = useState(undefined); - useEffect(() => { - waitForElement('#content-container-id').then(() => { - const mainDiv = document.getElementById('content-container-id'); - const height = mainDiv?.getBoundingClientRect().height - 59; - setEditorHeight(`${height}px`); - }); - }, []); - const getChangeHandler = () => { return debounce((value: string) => { setChangedTemplateBody(value); @@ -110,7 +100,7 @@ const WebhooksTemplateEditor: React.FC = ({ templat width="95%" >
-
+
= ({ templat value={template.value} data={{ payload_example: selectedPayload }} showLineNumbers={true} - height={editorHeight} + height="100%" onChange={getChangeHandler()} suggestionPrefix="" /> From c9a3b6757c15940c058dc4228ec2e1e003cf0f5e Mon Sep 17 00:00:00 2001 From: Dominik Broj Date: Fri, 1 Dec 2023 11:01:21 +0100 Subject: [PATCH 3/6] Fix inherit label toggler (#3472) # What this PR does Rely on local state instead of checked attribute when toggling inheritance ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/2352 ## 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) --- .../IntegrationLabelsForm.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx index b4e602ed..9d1703c7 100644 --- a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx +++ b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx @@ -68,13 +68,11 @@ const IntegrationLabelsForm = observer((props: IntegrationLabelsFormProps) => { onOpenIntegraionSettings(id); }; - const getInheritanceChangeHandler = (keyId: ApiSchemas['LabelKey']['id']) => { - return (event: React.ChangeEvent) => { - setAlertGroupLabels((alertGroupLabels) => ({ - ...alertGroupLabels, - inheritable: { ...alertGroupLabels.inheritable, [keyId]: event.target.checked }, - })); - }; + const onInheritanceChange = (keyId: ApiSchemas['LabelKey']['id']) => { + setAlertGroupLabels((alertGroupLabels) => ({ + ...alertGroupLabels, + inheritable: { ...alertGroupLabels.inheritable, [keyId]: !alertGroupLabels.inheritable[keyId] }, + })); }; return ( @@ -98,7 +96,7 @@ const IntegrationLabelsForm = observer((props: IntegrationLabelsFormProps) => { onInheritanceChange(label.key.id)} /> From 30caa18f9d22dfe52b25f8922437a85362c67df0 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Fri, 1 Dec 2023 18:49:00 +0800 Subject: [PATCH 4/6] Make telegram on_alert_group_action_triggered asynchronous (#3471) # What this PR does [send_alert_group_signal](https://github.com/grafana/oncall/blob/dev/engine/apps/alerts/tasks/send_alert_group_signal.py#L12) task is not idempotent. It launches [on_alert_group_action_triggered_async](https://github.com/grafana/oncall/blob/a2851d3f813c04768ce825c8af99e26912d80b4e/engine/apps/slack/representatives/alert_group_representative.py#L158) for slack and then might fail on [on_alert_group_action_triggered](https://github.com/grafana/oncall/blob/b2f4ffb98a940b894042df9161e209dc8a781dc1/engine/apps/telegram/alert_group_representative.py#L79) (not async) due to database DoesNotExist exception. This PR makes telegram representative asyncronous ## Which issue(s) this PR fixes ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 1 + .../telegram/alert_group_representative.py | 20 +++++++++---------- engine/apps/telegram/tasks.py | 18 +++++++++++++++++ engine/settings/celery_task_routes.py | 1 + 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e741b717..198b709b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Minor bugfixes + dependency updates :) - Filters polishing ([3183](https://github.com/grafana/oncall/issues/3183)) - Fixed permissions so User settings reader role included list users @mderynck ([#3419](https://github.com/grafana/oncall/pull/3419)) - Fixed alert group rendering when some links were broken because of replacing `-` to `_` @Ferril ([#3424](https://github.com/grafana/oncall/pull/3424)) +- Make telegram on_alert_group_action_triggered asynchronous([#3471](https://github.com/grafana/oncall/pull/3471)) ## v1.3.62 (2023-11-21) diff --git a/engine/apps/telegram/alert_group_representative.py b/engine/apps/telegram/alert_group_representative.py index a3b6933c..635420c6 100644 --- a/engine/apps/telegram/alert_group_representative.py +++ b/engine/apps/telegram/alert_group_representative.py @@ -3,7 +3,11 @@ import logging from apps.alerts.models import AlertGroup from apps.alerts.representative import AlertGroupAbstractRepresentative from apps.telegram.models import TelegramMessage -from apps.telegram.tasks import edit_message, on_create_alert_telegram_representative_async +from apps.telegram.tasks import ( + edit_message, + on_alert_group_action_triggered_async, + on_create_alert_telegram_representative_async, +) logger = logging.getLogger(__name__) logger.setLevel(logging.DEBUG) @@ -78,15 +82,11 @@ class AlertGroupTelegramRepresentative(AlertGroupAbstractRepresentative): from apps.alerts.models import AlertGroupLogRecord log_record = kwargs["log_record"] - logger.info(f"AlertGroupTelegramRepresentative ACTION SIGNAL, log record {log_record}") - - if not isinstance(log_record, AlertGroupLogRecord): - log_record = AlertGroupLogRecord.objects.get(pk=log_record) - - instance = cls(log_record) - if instance.is_applicable(): - handler = instance.get_handler() - handler() + if isinstance(log_record, AlertGroupLogRecord): + log_record_id = log_record.pk + else: + log_record_id = log_record + on_alert_group_action_triggered_async.apply_async((log_record_id,)) @staticmethod def on_create_alert(**kwargs): diff --git a/engine/apps/telegram/tasks.py b/engine/apps/telegram/tasks.py index 54b84945..1c160562 100644 --- a/engine/apps/telegram/tasks.py +++ b/engine/apps/telegram/tasks.py @@ -215,3 +215,21 @@ def on_create_alert_telegram_representative_async(self, alert_pk): ) for message in messages_to_edit: edit_message.delay(message_pk=message.pk) + + +@shared_dedicated_queue_retry_task( + autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None +) +def on_alert_group_action_triggered_async(log_record_id): + from apps.alerts.models import AlertGroupLogRecord + + from .alert_group_representative import AlertGroupTelegramRepresentative + + logger.info(f"AlertGroupTelegramRepresentative ACTION SIGNAL, log record {log_record_id}") + + log_record = AlertGroupLogRecord.objects.get(pk=log_record_id) + + instance = AlertGroupTelegramRepresentative(log_record) + if instance.is_applicable(): + handler = instance.get_handler() + handler() diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index b7803b46..a1f45836 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -161,6 +161,7 @@ CELERY_TASK_ROUTES = { "apps.telegram.tasks.register_telegram_webhook": {"queue": "telegram"}, "apps.telegram.tasks.send_link_to_channel_message_or_fallback_to_full_alert_group": {"queue": "telegram"}, "apps.telegram.tasks.send_log_and_actions_message": {"queue": "telegram"}, + "apps.telegram.tasks.on_alert_group_action_triggered_async": {"queue": "telegram"}, # WEBHOOK "apps.alerts.tasks.custom_button_result.custom_button_result": {"queue": "webhook"}, "apps.alerts.tasks.custom_webhook_result.custom_webhook_result": {"queue": "webhook"}, From 76a88bc0c15b37c80b685718fdead7ea2ab5869f Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Dec 2023 09:56:26 -0500 Subject: [PATCH 5/6] Revert "upgrade to Python 3.12 (#3456)" and "bump uwsgi version to latest #3466" (#3483) # What this PR does This reverts commits 7c4b40a0464a5813877ec08699b0167d1b87e90b and cdb22285dbe3a598d13efbc20ef31885b8759492. See https://github.com/grafana/oncall-private/pull/2361 for more details. --- .drone.yml | 6 ++-- .github/workflows/linting-and-tests.yml | 14 ++++---- .github/workflows/snyk.yml | 2 +- .pre-commit-config.yaml | 4 +-- CHANGELOG.md | 12 +++---- Makefile | 8 +++-- dev/README.md | 6 ++-- dev/scripts/generate-fake-data/README.md | 2 +- engine/Dockerfile | 2 +- .../grafana_alerting_sync.py | 4 +-- engine/apps/alerts/models/alert_group.py | 6 ++-- engine/apps/alerts/tests/test_paging.py | 34 ++++++------------- .../api/tests/test_alert_receive_channel.py | 4 +-- .../apps/api/views/alert_receive_channel.py | 4 +-- engine/apps/base/tests/test_live_settings.py | 2 +- .../tests/test_gcom_api_client.py | 2 +- .../apps/grafana_plugin/tests/test_install.py | 2 +- .../tests/test_self_hosted_install.py | 26 +++++++------- engine/apps/integrations/tests/test_views.py | 7 ++-- .../apps/oss_installation/cloud_heartbeat.py | 2 +- .../tests/test_phone_backend_call.py | 2 +- .../tests/test_phone_backend_sms.py | 2 +- .../public_api/tests/test_schedule_export.py | 4 +-- engine/apps/schedules/ical_utils.py | 8 ++--- .../apps/schedules/models/on_call_schedule.py | 5 ++- .../tests/test_shift_swap_request.py | 4 +-- .../scenarios/slack_channel_integration.py | 2 +- engine/apps/slack/scenarios/slack_renderer.py | 4 +-- .../tests/test_scenario_steps/test_paging.py | 12 ++----- engine/apps/slack/types/scenario_routes.py | 13 ++++++- .../apps/user_management/tests/test_region.py | 2 +- .../apps/user_management/tests/test_sync.py | 4 +-- engine/common/utils.py | 2 +- engine/pyproject.toml | 3 +- engine/requirements-dev.txt | 14 ++++---- engine/requirements.txt | 4 +-- tools/pagerduty-migrator/Dockerfile | 2 +- 37 files changed, 111 insertions(+), 125 deletions(-) diff --git a/.drone.yml b/.drone.yml index e9995481..dd1eaf7b 100644 --- a/.drone.yml +++ b/.drone.yml @@ -53,7 +53,7 @@ steps: - refs/tags/v*.*.* - name: Lint Backend - image: python:3.12.0 + image: python:3.11.4 environment: DJANGO_SETTINGS_MODULE: settings.ci-test commands: @@ -63,7 +63,7 @@ steps: - pre-commit run flake8 --all-files - name: Unit Test Backend - image: python:3.12.0 + image: python:3.11.4 environment: RABBITMQ_URI: amqp://rabbitmq:rabbitmq@rabbit_test:5672 DJANGO_SETTINGS_MODULE: settings.ci-test @@ -379,4 +379,4 @@ kind: secret name: github_api_token --- kind: signature -hmac: 36a7d2e2906bad4f186adfa488057ffb9107ef1cdbb3e4d7cb61165b00870c6b +hmac: b9e499a424faecd9a8f41552cc307bd3431cb0e3fac77f3ee99ce19258fc0fec diff --git a/.github/workflows/linting-and-tests.yml b/.github/workflows/linting-and-tests.yml index d90c3d5b..bc45b411 100644 --- a/.github/workflows/linting-and-tests.yml +++ b/.github/workflows/linting-and-tests.yml @@ -26,7 +26,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt @@ -117,7 +117,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt @@ -175,7 +175,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt @@ -225,7 +225,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt @@ -263,7 +263,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt @@ -282,7 +282,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: tools/pagerduty-migrator/requirements.txt - name: Unit Test PD Migrator @@ -298,7 +298,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: | engine/requirements.txt diff --git a/.github/workflows/snyk.yml b/.github/workflows/snyk.yml index 89470883..9d3ab192 100644 --- a/.github/workflows/snyk.yml +++ b/.github/workflows/snyk.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.12.0" + python-version: "3.11.4" cache: "pip" cache-dependency-path: engine/requirements.txt - uses: actions/setup-node@v3 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5a82df6a..eb1940b9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,7 +16,7 @@ repos: args: [--settings-file=dev/scripts/.isort.cfg, --filter-files] - repo: https://github.com/psf/black - rev: 23.11.0 + rev: 23.7.0 hooks: - id: black files: ^engine @@ -29,7 +29,7 @@ repos: files: ^dev/scripts - repo: https://github.com/pycqa/flake8 - rev: 6.1.0 + rev: 6.0.0 hooks: - id: flake8 files: ^engine diff --git a/CHANGELOG.md b/CHANGELOG.md index 198b709b..deaccc80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,11 +27,7 @@ Minor bugfixes + dependency updates :) ### Added -- Add options to customize table columns in AlertGroup page ([#3281](https://github.com/grafana/oncall/pull/3281)) - -### Changed - -- Upgrade to Python 3.12 by @joeyorlando ([#3456](https://github.com/grafana/oncall/pull/3456)) +- Add options to customize table columns in AlertGroup page ([3281](https://github.com/grafana/oncall/pull/3281)) ### Fixed @@ -49,9 +45,9 @@ Minor bugfixes + dependency updates :) ### Added - Add ability to use Grafana Service Account Tokens for OnCall API (This is only enabled for resolution_notes - endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189)) +endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189)) - Add ability for webhook presets to mask sensitive headers @mderynck - ([#3189](https://github.com/grafana/oncall/pull/3189)) +([#3189](https://github.com/grafana/oncall/pull/3189)) ### Changed @@ -60,7 +56,7 @@ Minor bugfixes + dependency updates :) ### Fixed - Fixed issue that blocked saving webhooks with presets if the preset is controlling the URL @mderynck - ([#3189](https://github.com/grafana/oncall/pull/3189)) +([#3189](https://github.com/grafana/oncall/pull/3189)) - User filter doesn't display current value on Alert Groups page ([1714](https://github.com/grafana/oncall/issues/1714)) - Remove displaying rotation modal for Terraform/API based schedules - Filters polishing ([3183](https://github.com/grafana/oncall/issues/3183)) diff --git a/Makefile b/Makefile index caa3cfa3..e4092de6 100644 --- a/Makefile +++ b/Makefile @@ -155,8 +155,12 @@ cleanup: stop ## this will remove all of the images, containers, volumes, and n docker system prune --filter label="$(DOCKER_COMPOSE_DEV_LABEL)" --all --volumes install-pre-commit: - echo "installing pre-commit" - pip install $$(grep "pre-commit" $(ENGINE_DIR)/requirements-dev.txt) + @if [ ! -x "$$(command -v pre-commit)" ]; then \ + echo "installing pre-commit"; \ + pip install $$(grep "pre-commit" $(ENGINE_DIR)/requirements-dev.txt); \ + else \ + echo "pre-commit already installed"; \ + fi lint: install-pre-commit ## run both frontend and backend linters ## may need to run `yarn install` from within `grafana-plugin` diff --git a/dev/README.md b/dev/README.md index b1a7303c..8b193e5c 100644 --- a/dev/README.md +++ b/dev/README.md @@ -58,7 +58,7 @@ Related: [How to develop integrations](/engine/config_integrations/README.md) ```yaml env: - name: FEATURE_LABELS_ENABLED_FOR_ALL - value: "True" + value: 'True' ``` 3. Wait until all resources are green and open (user: oncall, password: oncall) @@ -215,8 +215,8 @@ See the `django-silk` documentation [here](https://github.com/jazzband/django-si By default everything runs inside Docker. If you would like to run the backend services outside of Docker (for integrating w/ PyCharm for example), follow these instructions: -1. Create a Python 3.12 virtual environment using a method of your choosing (ex. - [venv](https://docs.python.org/3.12/library/venv.html) or [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)). +1. Create a Python 3.11 virtual environment using a method of your choosing (ex. + [venv](https://docs.python.org/3.11/library/venv.html) or [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)). Make sure the virtualenv is "activated". 2. `postgres` is a dependency on some of our Python dependencies (notably `psycopg2` ([docs](https://www.psycopg.org/docs/install.html#prerequisites))). Please visit diff --git a/dev/scripts/generate-fake-data/README.md b/dev/scripts/generate-fake-data/README.md index b0c04cb5..3607bc33 100644 --- a/dev/scripts/generate-fake-data/README.md +++ b/dev/scripts/generate-fake-data/README.md @@ -10,7 +10,7 @@ capable of generating the following objects: ## Prerequisites -1. Create/active a Python 3.12 virtual environment +1. Create/active a Python 3.11 virtual environment 2. `pip install -r requirements.txt` 3. Must have a local version of Grafana and OnCall up and running 4. Generate an API key inside of Grafana OnCall diff --git a/engine/Dockerfile b/engine/Dockerfile index 5a40ba67..e1b90cc5 100644 --- a/engine/Dockerfile +++ b/engine/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.12.0-alpine3.18 AS base +FROM python:3.11.4-alpine3.18 AS base # Create a group and user to run an app ENV APP_USER=appuser diff --git a/engine/apps/alerts/grafana_alerting_sync_manager/grafana_alerting_sync.py b/engine/apps/alerts/grafana_alerting_sync_manager/grafana_alerting_sync.py index e60e33e9..b0ed1558 100644 --- a/engine/apps/alerts/grafana_alerting_sync_manager/grafana_alerting_sync.py +++ b/engine/apps/alerts/grafana_alerting_sync_manager/grafana_alerting_sync.py @@ -203,7 +203,7 @@ class GrafanaAlertingSyncManager: if config is None: logger.warning( f"GrafanaAlertingSyncManager: Got config None in get_alerting_config_for_datasource " - f"for is_grafana_datasource {datasource_uid == cls.GRAFANA_ALERTING_DATASOURCE}, " + f"for is_grafana_datasource {datasource_uid==cls.GRAFANA_ALERTING_DATASOURCE}, " f"response: {response_info}" ) return @@ -232,7 +232,7 @@ class GrafanaAlertingSyncManager: if response is None: logger.warning( f"GrafanaAlertingSyncManager: Failed to update contact point (POST) for is_grafana_datasource " - f"{datasource_uid == cls.GRAFANA_ALERTING_DATASOURCE}; response: {response_info}" + f"{datasource_uid==cls.GRAFANA_ALERTING_DATASOURCE}; response: {response_info}" ) if response_info.get("status_code") == status.HTTP_400_BAD_REQUEST: logger.warning(f"GrafanaAlertingSyncManager: Config: {config}, Updated config: {updated_config}") diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 9d88410b..4a91a902 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1845,11 +1845,11 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. result_log_report = list() for log_record in log_records_list: - if type(log_record) is AlertGroupLogRecord: + if type(log_record) == AlertGroupLogRecord: result_log_report.append(log_record.render_log_line_json()) - elif type(log_record) is UserNotificationPolicyLogRecord: + elif type(log_record) == UserNotificationPolicyLogRecord: result_log_report.append(log_record.rendered_notification_log_line_json) - elif type(log_record) is ResolutionNote: + elif type(log_record) == ResolutionNote: result_log_report.append(log_record.render_log_line_json()) return result_log_report diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index aae3a8a5..2c0ae9f8 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -1,4 +1,4 @@ -from unittest.mock import call, patch +from unittest.mock import patch import pytest from django.utils import timezone @@ -74,16 +74,10 @@ def test_direct_paging_user(make_organization, make_user_for_organization): assert alert.message == msg # notifications sent - notifications_sent = ((user, False), (other_user, True)) - - notify_task.apply_async.assert_has_calls( - [ - call((u.pk, ag.pk), {"important": important, "notify_even_acknowledged": True, "notify_anyway": True}) - for u, important in notifications_sent - ] - ) - - for u, important in notifications_sent: + for u, important in ((user, False), (other_user, True)): + assert notify_task.apply_async.called_with( + (u.pk, ag.pk), {"important": important, "notify_even_acknowledged": True, "notify_anyway": True} + ) expected_info = {"user": u.public_primary_key, "important": important} assert_log_record(ag, f"{from_user.username} paged user {u.username}", expected_info=expected_info) @@ -179,7 +173,7 @@ def test_direct_paging_reusing_alert_group( # notifications sent ag = alert_groups.get() - notify_task.apply_async.assert_called_with( + assert notify_task.apply_async.called_with( (user.pk, ag.pk), {"important": False, "notify_even_acknowledged": True, "notify_anyway": True} ) @@ -249,17 +243,11 @@ def test_direct_paging_always_create_group(make_organization, make_user_for_orga assert alert_groups.count() == 2 # notifications sent - notify_task.apply_async.assert_has_calls( - [ - call( - (user.pk, alert_groups[0].pk), - {"important": False, "notify_even_acknowledged": True, "notify_anyway": True}, - ), - call( - (user.pk, alert_groups[1].pk), - {"important": False, "notify_even_acknowledged": True, "notify_anyway": True}, - ), - ] + assert notify_task.apply_async.called_with( + (user.pk, alert_groups[0].pk), {"important": False, "notify_even_acknowledged": True, "notify_anyway": True} + ) + assert notify_task.apply_async.called_with( + (user.pk, alert_groups[1].pk), {"important": False, "notify_even_acknowledged": True, "notify_anyway": True} ) diff --git a/engine/apps/api/tests/test_alert_receive_channel.py b/engine/apps/api/tests/test_alert_receive_channel.py index 308a1c39..8dac1b4b 100644 --- a/engine/apps/api/tests/test_alert_receive_channel.py +++ b/engine/apps/api/tests/test_alert_receive_channel.py @@ -85,10 +85,10 @@ def test_list_alert_receive_channel_skip_pagination_for_grafana_alerting( assert response.status_code == status.HTTP_200_OK if should_be_unpaginated: - assert type(results) is list + assert type(results) == list assert len(results) > 0 else: - assert type(results["results"]) is list + assert type(results["results"]) == list assert len(results["results"]) > 0 diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index 92ad3235..9edab2dc 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -271,7 +271,7 @@ class AlertReceiveChannelView( if payload is None: return channel.alert_groups.last().alerts.first() else: - if type(payload) is not dict: + if type(payload) != dict: raise PreviewTemplateException("Payload must be a valid json object") # Build Alert and AlertGroup objects to pass to templater without saving them to db alert_group_to_template = AlertGroup(channel=channel) @@ -336,7 +336,7 @@ class AlertReceiveChannelView( try: instance.start_maintenance(mode, duration, request.user) except MaintenanceCouldNotBeStartedError as e: - if type(instance) is AlertReceiveChannel: + if type(instance) == AlertReceiveChannel: detail = {"alert_receive_channel_id": ["Already on maintenance"]} else: detail = str(e) diff --git a/engine/apps/base/tests/test_live_settings.py b/engine/apps/base/tests/test_live_settings.py index e58d7060..965e2fb2 100644 --- a/engine/apps/base/tests/test_live_settings.py +++ b/engine/apps/base/tests/test_live_settings.py @@ -40,7 +40,7 @@ def test_multi_type_support(value): LiveSetting.objects.create(name="SOME_NEW_FEATURE_ENABLED", value=value) setting_value = LiveSetting.get_setting("SOME_NEW_FEATURE_ENABLED") - assert type(setting_value) is type(value) + assert type(setting_value) == type(value) assert setting_value == value diff --git a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py index 8e43388a..a8c3f67d 100644 --- a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py +++ b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py @@ -31,7 +31,7 @@ class TestIsRbacEnabledForStack: api_client = GcomAPIClient("someFakeApiToken") assert api_client.is_rbac_enabled_for_stack(stack_id) == expected - mocked_gcom_api_client_api_get.assert_called_once_with(f"instances/{stack_id}?config=true") + assert mocked_gcom_api_client_api_get.called_once_with(f"instances/{stack_id}?config=true") @pytest.mark.parametrize( "instance_info_feature_toggles,delimiter,expected", diff --git a/engine/apps/grafana_plugin/tests/test_install.py b/engine/apps/grafana_plugin/tests/test_install.py index 03471373..054f0156 100644 --- a/engine/apps/grafana_plugin/tests/test_install.py +++ b/engine/apps/grafana_plugin/tests/test_install.py @@ -20,7 +20,7 @@ def test_it_triggers_an_organization_sync_and_saves_the_grafana_token( response = client.post(reverse("grafana-plugin:install"), format="json", **auth_headers) assert response.status_code == status.HTTP_204_NO_CONTENT - mocked_sync_organization.assert_called_once_with(organization) + assert mocked_sync_organization.called_once_with(organization) # make sure api token is saved on the org organization.refresh_from_db() diff --git a/engine/apps/grafana_plugin/tests/test_self_hosted_install.py b/engine/apps/grafana_plugin/tests/test_self_hosted_install.py index b91d1174..a571dee4 100644 --- a/engine/apps/grafana_plugin/tests/test_self_hosted_install.py +++ b/engine/apps/grafana_plugin/tests/test_self_hosted_install.py @@ -74,8 +74,8 @@ def test_it_properly_handles_errors_from_the_grafana_api( url = reverse("grafana-plugin:self-hosted-install") response = client.post(url, format="json", **make_self_hosted_install_header(GRAFANA_TOKEN)) - mocked_grafana_api_client.assert_called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) - mocked_grafana_api_client.return_value.check_token.assert_called_once_with() + assert mocked_grafana_api_client.called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) + assert mocked_grafana_api_client.return_value.check_token.called_once_with() assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.data["error"] == expected_error_msg @@ -106,13 +106,13 @@ def test_if_organization_exists_it_is_updated( url = reverse("grafana-plugin:self-hosted-install") response = client.post(url, format="json", **make_self_hosted_install_header(GRAFANA_TOKEN)) - mocked_grafana_api_client.assert_called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) - mocked_grafana_api_client.return_value.check_token.assert_called_once_with() - mocked_grafana_api_client.return_value.is_rbac_enabled_for_organization.assert_called_once_with() + assert mocked_grafana_api_client.called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) + assert mocked_grafana_api_client.return_value.check_token.called_once_with() + assert mocked_grafana_api_client.return_value.is_rbac_enabled_for_organization.called_once_with() - mocked_sync_organization.assert_called_once_with(organization) - mocked_provision_plugin.assert_called_once_with() - mocked_revoke_plugin.assert_called_once_with() + assert mocked_sync_organization.called_once_with(organization) + assert mocked_provision_plugin.called_once_with() + assert mocked_revoke_plugin.called_once_with() assert response.status_code == status.HTTP_201_CREATED assert response.data == {"error": None, **provision_plugin_response} @@ -151,12 +151,12 @@ def test_if_organization_does_not_exist_it_is_created( organization = Organization.objects.filter(stack_id=STACK_ID, org_id=ORG_ID).first() - mocked_grafana_api_client.assert_called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) - mocked_grafana_api_client.return_value.check_token.assert_called_once_with() - mocked_grafana_api_client.return_value.is_rbac_enabled_for_organization.assert_called_once_with() + assert mocked_grafana_api_client.called_once_with(api_url=GRAFANA_API_URL, api_token=GRAFANA_TOKEN) + assert mocked_grafana_api_client.return_value.check_token.called_once_with() + assert mocked_grafana_api_client.return_value.is_rbac_enabled_for_organization.called_once_with() - mocked_sync_organization.assert_called_once_with(organization) - mocked_provision_plugin.assert_called_once_with() + assert mocked_sync_organization.called_once_with(organization) + assert mocked_provision_plugin.called_once_with() assert not mocked_revoke_plugin.called assert response.status_code == status.HTTP_201_CREATED diff --git a/engine/apps/integrations/tests/test_views.py b/engine/apps/integrations/tests/test_views.py index a178d57b..80274a62 100644 --- a/engine/apps/integrations/tests/test_views.py +++ b/engine/apps/integrations/tests/test_views.py @@ -4,7 +4,7 @@ import pytest from django.core.files.uploadedfile import SimpleUploadedFile from django.db import OperationalError from django.urls import reverse -from pytest_django import DjangoDbBlocker +from pytest_django.plugin import _DatabaseBlocker from rest_framework import status from rest_framework.test import APIClient @@ -12,12 +12,9 @@ from apps.alerts.models import AlertReceiveChannel from apps.integrations.mixins import AlertChannelDefiningMixin -class DatabaseBlocker(DjangoDbBlocker): +class DatabaseBlocker(_DatabaseBlocker): """Customize pytest_django db blocker to raise OperationalError exception.""" - def __init__(self, *args) -> None: - super().__init__(_ispytest=True) - def _blocking_wrapper(*args, **kwargs): __tracebackhide__ = True __tracebackhide__ # Silence pyflakes diff --git a/engine/apps/oss_installation/cloud_heartbeat.py b/engine/apps/oss_installation/cloud_heartbeat.py index b42fee6b..82b68a64 100644 --- a/engine/apps/oss_installation/cloud_heartbeat.py +++ b/engine/apps/oss_installation/cloud_heartbeat.py @@ -47,7 +47,7 @@ def setup_heartbeat_integration(name=None): } ) else: - setup_heartbeat_integration(f"{name} {random.randint(1, 1024)}") + setup_heartbeat_integration(f"{name} { random.randint(1, 1024)}") except requests.Timeout: logger.warning("Unable to create cloud heartbeat integration. Request timeout.") except requests.exceptions.RequestException as e: diff --git a/engine/apps/phone_notifications/tests/test_phone_backend_call.py b/engine/apps/phone_notifications/tests/test_phone_backend_call.py index 0e69776b..cab9e406 100644 --- a/engine/apps/phone_notifications/tests/test_phone_backend_call.py +++ b/engine/apps/phone_notifications/tests/test_phone_backend_call.py @@ -141,7 +141,7 @@ def test_notify_by_provider_call_limits_warning( phone_backend = PhoneBackend() phone_backend._notify_by_provider_call(user, "some_message") - mock_add_call_limit_warning.assert_called_once_with(2, "some_message") + assert mock_add_call_limit_warning.called_once_with(2, "some_message") @pytest.mark.django_db diff --git a/engine/apps/phone_notifications/tests/test_phone_backend_sms.py b/engine/apps/phone_notifications/tests/test_phone_backend_sms.py index ff9b07aa..ec414ad5 100644 --- a/engine/apps/phone_notifications/tests/test_phone_backend_sms.py +++ b/engine/apps/phone_notifications/tests/test_phone_backend_sms.py @@ -150,7 +150,7 @@ def test_notify_by_provider_sms_limits_warning( phone_backend = PhoneBackend() phone_backend._notify_by_provider_sms(user, "some_message") - mock_add_sms_limit_warning.assert_called_once_with(2, "some_message") + assert mock_add_sms_limit_warning.called_once_with(2, "some_message") @pytest.mark.django_db diff --git a/engine/apps/public_api/tests/test_schedule_export.py b/engine/apps/public_api/tests/test_schedule_export.py index 559a4a6c..9a5835f1 100644 --- a/engine/apps/public_api/tests/test_schedule_export.py +++ b/engine/apps/public_api/tests/test_schedule_export.py @@ -78,7 +78,7 @@ def test_export_calendar(make_organization_and_user_with_token, make_user_for_or cal = Calendar.from_ical(response.data) - assert type(cal) is Calendar + assert type(cal) == Calendar # check there are events assert len(cal.subcomponents) > 0 for component in cal.walk(): @@ -112,7 +112,7 @@ def test_export_user_calendar(make_organization_and_user_with_token, make_schedu cal = Calendar.from_ical(response.data) - assert type(cal) is Calendar + assert type(cal) == Calendar assert cal.get("x-wr-calname") == "On-Call Schedule for {0}".format(user.username) assert cal.get("x-wr-timezone") == "UTC" assert cal.get("calscale") == "GREGORIAN" diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 9c96ec46..e39874fd 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -184,7 +184,7 @@ def list_of_oncall_shifts_from_ical( pytz_tz = pytz.timezone("UTC") return ( datetime.datetime.combine(e["start"], datetime.datetime.min.time(), tzinfo=pytz_tz) - if type(e["start"]) is datetime.date + if type(e["start"]) == datetime.date else e["start"] ) @@ -231,7 +231,7 @@ def get_shifts_dict( ) # Define on-call shift out of ical event that has the actual user if len(users) > 0 or with_empty_shifts: - if type(event[ICAL_DATETIME_START].dt) is datetime.date: + if type(event[ICAL_DATETIME_START].dt) == datetime.date: start = event[ICAL_DATETIME_START].dt end = event[ICAL_DATETIME_END].dt result_date.append( @@ -623,7 +623,7 @@ def is_icals_equal(first, second): def ical_date_to_datetime(date, tz, start): datetime_to_combine = datetime.time.min all_day = False - if type(date) is datetime.date: + if type(date) == datetime.date: all_day = True calendar_timezone_offset = datetime.datetime.now().astimezone(tz).utcoffset() date = datetime.datetime.combine(date, datetime_to_combine).astimezone(tz) - calendar_timezone_offset @@ -776,7 +776,7 @@ def start_end_with_respect_to_all_day(event: IcalEvent, calendar_tz): def event_start_end_all_day_with_respect_to_type(event: IcalEvent, calendar_tz): all_day = False - if type(event[ICAL_DATETIME_START].dt) is datetime.date: + if type(event[ICAL_DATETIME_START].dt) == datetime.date: start, end = start_end_with_respect_to_all_day(event, calendar_tz) all_day = True else: diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 671b99a8..f3bb8342 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -375,7 +375,7 @@ class OnCallSchedule(PolymorphicModel): events: ScheduleEvents = [] for shift in shifts: start = shift["start"] - all_day = type(start) is datetime.date + all_day = type(start) == datetime.date # fix confusing end date for all-day event end = shift["end"] - datetime.timedelta(days=1) if all_day else shift["end"] if all_day and all_day_datetime: @@ -500,7 +500,7 @@ class OnCallSchedule(PolymorphicModel): # check if event was ended or cancelled, update ical dtend = component.get(ICAL_DATETIME_END) dtend_datetime = dtend.dt if dtend else None - if dtend_datetime and type(dtend_datetime) is datetime.date: + if dtend_datetime and type(dtend_datetime) == datetime.date: # shift or overrides coming from ical calendars can be all day events, change to datetime dtend_datetime = datetime.datetime.combine( dtend.dt, datetime.datetime.min.time(), tzinfo=pytz.UTC @@ -1120,7 +1120,6 @@ class OnCallScheduleICal(OnCallSchedule): class OnCallScheduleCalendar(OnCallSchedule): - custom_on_call_shifts: "RelatedManager['CustomOnCallShift']" escalation_policies: "RelatedManager['EscalationPolicy']" objects: models.Manager["OnCallScheduleCalendar"] schedule_export_token: "RelatedManager['ScheduleExportAuthToken']" diff --git a/engine/apps/schedules/tests/test_shift_swap_request.py b/engine/apps/schedules/tests/test_shift_swap_request.py index e87bb156..ab1d87c3 100644 --- a/engine/apps/schedules/tests/test_shift_swap_request.py +++ b/engine/apps/schedules/tests/test_shift_swap_request.py @@ -23,7 +23,7 @@ def test_soft_delete(shift_swap_request_setup): ssr.refresh_from_db() assert ssr.deleted_at is not None - mock_refresh_final.apply_async.assert_called_with((ssr.schedule.pk,)) + assert mock_refresh_final.apply_async.called_with((ssr.schedule.pk,)) assert ShiftSwapRequest.objects.all().count() == 0 assert ShiftSwapRequest.objects_with_deleted.all().count() == 1 @@ -100,7 +100,7 @@ def test_take( mock_notify_beneficiary_about_taken_shift_swap_request.apply_async.assert_called_once_with((ssr.pk,)) # final schedule refresh was triggered - mock_refresh_final.apply_async.assert_called_with((ssr.schedule.pk,)) + assert mock_refresh_final.apply_async.called_with((ssr.schedule.pk,)) @pytest.mark.django_db diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index a4d1f637..234ea53c 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -142,7 +142,7 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): STEPS_ROUTING: ScenarioRoute.RoutingSteps = [ typing.cast( - ScenarioRoute.EventCallbackScenarioRoute, + ScenarioRoute.EventCallbackChannelMessageScenarioRoute, { "payload_type": PayloadType.EVENT_CALLBACK, "event_type": EventType.MESSAGE, diff --git a/engine/apps/slack/scenarios/slack_renderer.py b/engine/apps/slack/scenarios/slack_renderer.py index 982671a7..853f7372 100644 --- a/engine/apps/slack/scenarios/slack_renderer.py +++ b/engine/apps/slack/scenarios/slack_renderer.py @@ -22,9 +22,9 @@ class AlertGroupLogSlackRenderer: # get rendered logs result = "" for log_record in all_log_records: # list of AlertGroupLogRecord and UserNotificationPolicyLogRecord logs - if type(log_record) is AlertGroupLogRecord: + if type(log_record) == AlertGroupLogRecord: result += f"{log_record.rendered_incident_log_line(for_slack=True)}\n" - elif type(log_record) is UserNotificationPolicyLogRecord: + elif type(log_record) == UserNotificationPolicyLogRecord: result += f"{log_record.rendered_notification_log_line(for_slack=True)}\n" attachments.append( diff --git a/engine/apps/slack/tests/test_scenario_steps/test_paging.py b/engine/apps/slack/tests/test_scenario_steps/test_paging.py index 646149d4..64c0c29e 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_paging.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_paging.py @@ -242,9 +242,7 @@ def test_trigger_paging_additional_responders(make_organization_and_user_with_sl with patch.object(step._slack_client, "api_call"): step.process_scenario(slack_user_identity, slack_team_identity, payload) - mock_direct_paging.assert_called_once_with( - organization=organization, from_user=user, message="The Message", team=team, users=[(user, True)] - ) + mock_direct_paging.called_once_with(organization, user, "The Message", team, [(user, True)]) @pytest.mark.django_db @@ -258,13 +256,7 @@ def test_page_team(make_organization_and_user_with_slack_identities, make_team): with patch.object(step._slack_client, "api_call"): step.process_scenario(slack_user_identity, slack_team_identity, payload) - mock_direct_paging.assert_called_once_with( - organization=organization, - from_user=user, - message="The Message", - team=team, - users=[], - ) + mock_direct_paging.called_once_with(organization, user, "The Message", team) @pytest.mark.django_db diff --git a/engine/apps/slack/types/scenario_routes.py b/engine/apps/slack/types/scenario_routes.py index 503a62ef..f9928fe4 100644 --- a/engine/apps/slack/types/scenario_routes.py +++ b/engine/apps/slack/types/scenario_routes.py @@ -19,7 +19,17 @@ class ScenarioRoute: class EventCallbackScenarioRoute(_Base): payload_type: typing.Literal[PayloadType.EVENT_CALLBACK] event_type: EventType - message_channel_type: typing.NotRequired[typing.Literal[EventType.MESSAGE_CHANNEL]] + + class EventCallbackChannelMessageScenarioRoute(EventCallbackScenarioRoute): + """ + NOTE: the reason why we need to subclass `EventCallbackScenarioRoute` is because in Python 3.11 there is currently + no way to specify keys as optional in a `typing.TypedDict`. See [PEP-692](https://peps.python.org/pep-0692/) which + will implement this typing feature in Python 3.12. + + When we upgrade to 3.12 we should update this type. + """ + + message_channel_type: typing.Literal[EventType.MESSAGE_CHANNEL] class InteractiveMessageScenarioRoute(_Base): payload_type: typing.Literal[PayloadType.INTERACTIVE_MESSAGE] @@ -41,6 +51,7 @@ class ScenarioRoute: RoutingStep = ( BlockActionsScenarioRoute | EventCallbackScenarioRoute + | EventCallbackChannelMessageScenarioRoute | InteractiveMessageScenarioRoute | MessageActionScenarioRoute | SlashCommandScenarioRoute diff --git a/engine/apps/user_management/tests/test_region.py b/engine/apps/user_management/tests/test_region.py index d62e4033..8aa0e7b3 100644 --- a/engine/apps/user_management/tests/test_region.py +++ b/engine/apps/user_management/tests/test_region.py @@ -270,7 +270,7 @@ def test_organization_moved_middleware_amazon_sns_headers( response = client.post(url, data, format="json", **expected_sns_headers) assert mocked_make_request.called for k in AMAZON_SNS_HEADERS: - assert expected_sns_headers.get(f'HTTP_{k.upper().replace("-", "_")}') == mocked_make_request.call_args.args[ + assert expected_sns_headers.get(f'HTTP_{k.upper().replace("-","_")}') == mocked_make_request.call_args.args[ 2 ].get(k) assert response.content == expected_message diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 505f6a3c..86d4881b 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -368,8 +368,8 @@ def test_sync_organization_is_rbac_permissions_enabled_cloud(mocked_gcom_client, organization.refresh_from_db() - mocked_gcom_client.assert_called_once_with("mockedToken") - mocked_gcom_client.return_value.is_rbac_enabled_for_stack.assert_called_once_with(stack_id) + assert mocked_gcom_client.return_value.called_once_with("mockedToken") + assert mocked_gcom_client.return_value.is_rbac_enabled_for_stack.called_once_with(stack_id) assert organization.is_rbac_permissions_enabled == gcom_api_response diff --git a/engine/common/utils.py b/engine/common/utils.py index f3f56507..7b156636 100644 --- a/engine/common/utils.py +++ b/engine/common/utils.py @@ -150,7 +150,7 @@ def isoformat_with_tz_suffix(value): def is_string_with_visible_characters(string): - return type(string) is str and not string.isspace() and not string == "" + return type(string) == str and not string.isspace() and not string == "" def str_or_backup(string, backup): diff --git a/engine/pyproject.toml b/engine/pyproject.toml index 2a03281a..d1b94fde 100644 --- a/engine/pyproject.toml +++ b/engine/pyproject.toml @@ -2,13 +2,12 @@ profile = "black" line_length=120 float_to_top=true -# TODO: update py_version to 312 once isort supports it py_version=311 extend_skip_glob = "**/migrations/**" [tool.black] line-length = 120 -target-version = ["py312"] +target-version = ["py311"] force-exclude = "migrations" [tool.mypy] diff --git a/engine/requirements-dev.txt b/engine/requirements-dev.txt index 589bbaca..4c9577a7 100644 --- a/engine/requirements-dev.txt +++ b/engine/requirements-dev.txt @@ -1,12 +1,12 @@ celery-types==0.18.0 django-filter-stubs==0.1.3 -django-stubs==4.2.6 -djangorestframework-stubs==3.14.4 -mypy==1.7.1 -pre-commit==3.5.0 -pytest==7.4.3 -pytest-django==4.7.0 -pytest_factoryboy==2.6.0 +django-stubs[compatible-mypy]==4.2.2 +djangorestframework-stubs[compatible-mypy]==3.14.2 +mypy==1.4.1 +pre-commit==2.15.0 +pytest==7.3.1 +pytest-django==4.5.2 +pytest_factoryboy==2.5.1 types-beautifulsoup4==4.12.0.5 types-PyMySQL==1.0.19.7 types-python-dateutil==2.8.19.13 diff --git a/engine/requirements.txt b/engine/requirements.txt index a22ce860..061ea14f 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -50,7 +50,7 @@ pymdown-extensions==10.0 requests==2.31.0 urllib3==1.26.18 prometheus_client==0.16.0 -lxml==4.9.3 +lxml==4.9.2 babel==2.12.1 drf-spectacular==0.26.5 -grpcio==1.59.0 +grpcio==1.57.0 diff --git a/tools/pagerduty-migrator/Dockerfile b/tools/pagerduty-migrator/Dockerfile index 21ba3555..3405d29d 100644 --- a/tools/pagerduty-migrator/Dockerfile +++ b/tools/pagerduty-migrator/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.12.0-alpine +FROM python:3.11.4-alpine ENV PYTHONUNBUFFERED=1 WORKDIR /app From 34bab57dcdf829bafa982d822366cb7f1a022c10 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Dec 2023 10:05:24 -0500 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index deaccc80..cbe7c64f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.69 (2023-12-01) + +Maintenance release + bugfixes + ## v1.3.68 (2023-11-30) ### Fixed