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

ref(grouping): Do preliminary typing work for enhancements #87369

Draft
wants to merge 9 commits into
base: kmclb-small-enhancement-simplifications
Choose a base branch
from

Conversation

lobsterkatie
Copy link
Member

As a first step towards adding types to the enhancements code, this makes a handful of code changes which will be necessary once types are added to all of the enhancements methods, which will happen in a follow-up PR. (I split it up this way so that hopefully it will be a little easier to review.)

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 18, 2025
Copy link

codecov bot commented Mar 19, 2025

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
24462 3 24459 299
View the top 3 failed test(s) by shortest run time
tests.sentry.workflow_engine.migration_helpers.test_migrate_alert_rule.SinglePointOfEntryTest::test_spe_create
Stack Traces | 3.13s run time
#x1B[1m#x1B[.../workflow_engine/migration_helpers/test_migrate_alert_rule.py#x1B[0m:1330: in test_spe_create
    dual_write_alert_rule(self.metric_alert)
#x1B[1m#x1B[.../workflow_engine/migration_helpers/alert_rule.py#x1B[0m:593: in dual_write_alert_rule
    migrate_alert_rule(alert_rule)
#x1B[1m#x1B[.../workflow_engine/migration_helpers/alert_rule.py#x1B[0m:552: in migrate_alert_rule
    workflow_name = get_workflow_name(alert_rule)
#x1B[1m#x1B[.../workflow_engine/migration_helpers/utils.py#x1B[0m:53: in get_workflow_name
    description = get_action_description(action) + ", "
#x1B[1m#x1B[.../workflow_engine/migration_helpers/utils.py#x1B[0m:23: in get_action_description
    if action.target:
#x1B[1m#x1B[.../incidents/models/alert_rule.py#x1B[0m:476: in target
    return user_service.get_user(user_id=int(self.target_identifier))
#x1B[1m#x1B[.../services/user/service.py#x1B[0m:201: in get_user
    return get_user(user_id)
#x1B[1m#x1B[.../rpc/caching/service.py#x1B[0m:76: in __call__
    return self.get_one(object_id)
#x1B[1m#x1B[.../rpc/caching/service.py#x1B[0m:109: in get_one
    return _consume_generator(self.resolve_from(object_id, values))
#x1B[1m#x1B[.../rpc/caching/impl.py#x1B[0m:24: in _consume_generator
    g.send(None)
#x1B[1m#x1B[.../rpc/caching/service.py#x1B[0m:99: in resolve_from
    r = self.cb(i)
#x1B[1m#x1B[.../services/user/service.py#x1B[0m:320: in get_user
    users = user_service.get_many(filter={"user_ids": [user_id]})
#x1B[1m#x1B[.../hybridcloud/rpc/service.py#x1B[0m:354: in remote_method
    return dispatch_remote_call(
#x1B[1m#x1B[.../hybridcloud/rpc/service.py#x1B[0m:476: in dispatch_remote_call
    return remote_silo_call.dispatch(use_test_client)
#x1B[1m#x1B[.../hybridcloud/rpc/service.py#x1B[0m:511: in dispatch
    serial_response = self._send_to_remote_silo(use_test_client)
#x1B[1m#x1B[.../hybridcloud/rpc/service.py#x1B[0m:572: in _send_to_remote_silo
    response = self._fire_test_request(headers, data)
#x1B[1m#x1B[.../hybridcloud/rpc/service.py#x1B[0m:631: in _fire_test_request
    in_test_assert_no_transaction(
#x1B[1m#x1B[.../db/postgres/transactions.py#x1B[0m:101: in in_test_assert_no_transaction
    assert not hybrid_cloud.simulated_transaction_watermarks.connection_transaction_depth_above_watermark(
#x1B[1m#x1B[31mE   AssertionError: remote service method to .../rpc/user/get_many/ called inside transaction!  Move service calls to outside of transactions.#x1B[0m
tests.sentry.workflow_engine.handlers.action.notification.test_metric_alert.TestPagerDutyMetricAlertHandler::test_invoke_legacy_registry
Stack Traces | 3.5s run time
#x1B[1m#x1B[.../action/notification/test_metric_alert.py#x1B[0m:387: in test_invoke_legacy_registry
    self.assert_notification_context(
#x1B[1m#x1B[.../action/notification/test_metric_alert.py#x1B[0m:82: in assert_notification_context
    assert asdict(notification_context) == {
#x1B[1m#x1B[31mE   AssertionError: assert {'id': 8, 'in...d': None, ...} == {'integration...y': None, ...}#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     Omitting 5 identical items, use -vv to show#x1B[0m
#x1B[1m#x1B[31mE     Left contains 1 more item:#x1B[0m
#x1B[1m#x1B[31mE     {'id': 8}#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     Full diff:#x1B[0m
#x1B[1m#x1B[31mE       {#x1B[0m
#x1B[1m#x1B[31mE     +     'id': 8,#x1B[0m
#x1B[1m#x1B[31mE           'integration_id': 1234567890,#x1B[0m
#x1B[1m#x1B[31mE           'sentry_app_config': {#x1B[0m
#x1B[1m#x1B[31mE               'priority': 'P1',#x1B[0m
#x1B[1m#x1B[31mE           },#x1B[0m
#x1B[1m#x1B[31mE           'sentry_app_id': None,#x1B[0m
#x1B[1m#x1B[31mE           'target_display': None,#x1B[0m
#x1B[1m#x1B[31mE           'target_identifier': 'service123',#x1B[0m
#x1B[1m#x1B[31mE       }#x1B[0m
tests.sentry.workflow_engine.handlers.action.notification.test_metric_alert.TestBaseMetricAlertHandler::test_invoke_legacy_registry
Stack Traces | 3.54s run time
#x1B[1m#x1B[.../action/notification/test_metric_alert.py#x1B[0m:277: in test_invoke_legacy_registry
    self.assert_notification_context(
#x1B[1m#x1B[.../action/notification/test_metric_alert.py#x1B[0m:82: in assert_notification_context
    assert asdict(notification_context) == {
#x1B[1m#x1B[31mE   AssertionError: assert {'id': 25, 'i...d': None, ...} == {'integration...y': None, ...}#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     Omitting 5 identical items, use -vv to show#x1B[0m
#x1B[1m#x1B[31mE     Left contains 1 more item:#x1B[0m
#x1B[1m#x1B[31mE     {'id': 25}#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     Full diff:#x1B[0m
#x1B[1m#x1B[31mE       {#x1B[0m
#x1B[1m#x1B[31mE     +     'id': 25,#x1B[0m
#x1B[1m#x1B[31mE           'integration_id': '1234567890',#x1B[0m
#x1B[1m#x1B[31mE           'sentry_app_config': None,#x1B[0m
#x1B[1m#x1B[31mE           'sentry_app_id': None,#x1B[0m
#x1B[1m#x1B[31mE           'target_display': None,#x1B[0m
#x1B[1m#x1B[31mE           'target_identifier': 'channel456',#x1B[0m
#x1B[1m#x1B[31mE       }#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@lobsterkatie lobsterkatie force-pushed the kmclb-small-enhancement-simplifications branch from 49bf1f3 to cb18d48 Compare March 19, 2025 20:29
@lobsterkatie lobsterkatie force-pushed the kmclb-enhancement-types-prework branch from 2760276 to 4eb7eb8 Compare March 19, 2025 20:30
@lobsterkatie lobsterkatie force-pushed the kmclb-small-enhancement-simplifications branch from cb18d48 to a3e3a19 Compare March 19, 2025 21:23
@lobsterkatie lobsterkatie force-pushed the kmclb-enhancement-types-prework branch from 4eb7eb8 to 0c66767 Compare March 19, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants