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

feat(types): Add NonNone non-null assertion operator stand-in #70236

Merged
merged 1 commit into from
May 3, 2024

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented May 3, 2024

This introduces a NonNone type utility meant to mimic the non-null assertion operator (!) in TypeScript.

For example, given a function with the signature make_stuff() -> Stuff | None, used in a place where you know it's going to return a Stuff instance, you can change your call from stuff = make_stuff() to stuff = NonNone(make_stuff()), and mypy will stop complaining that stuff might be None.

I know you can do this with cast, but this seemed more expressive and like it would add a little less( or possibly a lot less) clutter to the code. (Consider a value typed as dict[str, tuple[list[Stuff]]] | None. Would you rather cast(dict[str, tuple[list[Stuff]]], stuff) or NonNone(stuff)?)

Disclaimer: This may or may not be the most elegant way to do this, and I'm very open to suggestions for future improvements (or for a different approach entirely), but for now this clears a blocker (and was the best option with which fairly extensive googling provided me), so I went with it.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 3, 2024
@lobsterkatie lobsterkatie requested a review from jangjodi May 3, 2024 17:25
Copy link
Member

@jangjodi jangjodi left a comment

Choose a reason for hiding this comment

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

this lgtm, though I'm not not the most familiar with typing/other ways to do this. might benefit from another set of eyes

@jangjodi jangjodi requested a review from a team May 3, 2024 17:38
Copy link
Member

@snigdhas snigdhas left a comment

Choose a reason for hiding this comment

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

yay for stronger typing!

@vartec
Copy link
Contributor

vartec commented May 3, 2024

FWIW, you can achieve similar result with typing.cast

stuff = cast(T, make_stuff())

@lobsterkatie
Copy link
Member Author

FWIW, you can achieve similar result with typing.cast

stuff = cast(T, make_stuff())

Yup, I know - this just seemed more expressive and (slightly) less likely to add clutter all over my code. But it's a good point - I'll add a note addressing this in the PR description.

@lobsterkatie lobsterkatie force-pushed the kmclb-add-NonNone-type-helper branch from 63bd554 to 62e6dc4 Compare May 3, 2024 22:12
@lobsterkatie lobsterkatie marked this pull request as ready for review May 3, 2024 22:44
@lobsterkatie lobsterkatie merged commit 3e90887 into master May 3, 2024
48 checks passed
@lobsterkatie lobsterkatie deleted the kmclb-add-NonNone-type-helper branch May 3, 2024 22:44
lobsterkatie added a commit that referenced this pull request May 7, 2024
This adds the group hash to the outgoing payload sent to Seer. Once this is merged, it will be safe to remove the Seer logic handling an incoming group id, since we'll now be able to rely on the group hash being present. 

Taken together, this PR and the ones leading up to it (links below) mean will mean that on the Sentry side we'll be sending in the request and handling in the response both hash and group id (and transforming hashes in the response into group ids if hashes are all that's sent). Once Seer is adjusted to only deal in hashes, we can then remove the sending and handling of group ids from Sentry and the transition will be complete.

Other PRs which are part of the group-id-to-group-hash switch:
- #70005, #70236, and #70237 - various small fixes and tweaks
- #70070 and #70238 - updates to associated types
- #70240 - automatic conversion from hash to group id when handling Seer similar group data
iamrajjoshi added a commit that referenced this pull request May 8, 2024
commit 1354478
Author: Raj Joshi <raj.joshi@sentry.io>
Date:   Wed May 8 09:56:24 2024 -0700

    feat(chartcuterie): Change Chart Stylings (#70489)

    two quick updates:

    1. Changes the label for regressed to a darker color so we can read it
    better

    ![image](https://github.com/getsentry/sentry/assets/33237075/80cd09dd-3619-4d58-abd3-65795de248c0)

    2. Updated some chart styling for slack so the legend is on the left

    ![image](https://github.com/getsentry/sentry/assets/33237075/de5a048a-cad5-4e9a-b152-15cbf9bc254e)

commit 56514bc
Author: Catherine Lee <55311782+c298lee@users.noreply.github.com>
Date:   Wed May 8 12:47:19 2024 -0400

    ref(replay): Rage click clicked element name (#70493)

    Since we now use react component names in the selector path, we should
    modify clicked element to provide more specific info such as class, role
    etc.
    This prevents giving the react component name twice and would give more
    info for debugging since react component names aren't very specific

    Before:
    <img width="845" alt="image"
    src="https://github.com/getsentry/sentry/assets/55311782/87b6c8cd-1b29-40bb-b448-715595cb255b">

commit 35b38d9
Author: Steven Eubank <47563310+smeubank@users.noreply.github.com>
Date:   Wed May 8 18:38:57 2024 +0200

    Add Deno Runtime Icon (#69100)

    QoL, this has been bugging me

    Should show the Deno icon, from Deno runtimes hosted on Supabase and
    others

    after:

    ![image](https://github.com/getsentry/sentry/assets/47563310/a1c63d17-3a89-4639-93d1-bca250f0bc31)

    ![image](https://github.com/getsentry/sentry/assets/47563310/ca66d7cd-8315-4366-a828-8a93936c1a07)

commit a96f1ff
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Wed May 8 12:34:37 2024 -0400

    ref(crons): Improve details legend (#70515)

    The legend now differentiates between timeout and failed

    <img alt="clipboard.png" width="267"
    src="https://i.imgur.com/yQAfXaB.png" />

    ---------

    Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

commit c3b9fdb
Author: Matt Duncan <14761+mrduncan@users.noreply.github.com>
Date:   Wed May 8 09:23:28 2024 -0700

    chore(issues): Enable stronger typing on two endpoints (#70488)

    This is another quick follow up to #69828 since these two type errors
    are trivial to resolve.

commit dbc926a
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Wed May 8 12:22:14 2024 -0400

    ref(crons): Move timezone to schedule text (#70511)

    <img width="295" alt="image"
    src="https://github.com/getsentry/sentry/assets/1421724/fd919e5b-0dd6-4b41-8e84-d3b99dfe6042">

    It is now next to the schedule. This is more logical

commit 9a90b57
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Wed May 8 12:18:49 2024 -0400

    ref(crons): Use constant for DEFAULT_CHECKIN_MARGIN (#70510)

commit f161ebb
Author: Colleen O'Rourke <colleen@sentry.io>
Date:   Wed May 8 09:16:58 2024 -0700

    ref(daily summary): Disable notification (#70295)

    I don't currently have any time to dedicate to bug fixing and it's been
    sending multiple times, so this is going to be disabled for now. We'll
    likely have to keep track of which users per org received the
    notification and check that before sending, and then clear it out every
    hour to avoid duplicate sending.

commit 7d1cf85
Author: Matt Duncan <14761+mrduncan@users.noreply.github.com>
Date:   Wed May 8 08:58:45 2024 -0700

    chore(issues): Enable stronger typing on occurrence_consumer (#70487)

    This is a quick follow up to #69828 since these two type errors are
    trivial to resolve.

commit 6b530ff
Author: colin-sentry <161344340+colin-sentry@users.noreply.github.com>
Date:   Wed May 8 11:50:48 2024 -0400

    chore(ai-monitoring): Add a unit to total cost (#70484)

commit 50b4ed3
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Wed May 8 11:32:28 2024 -0400

    ref(routes): A few more routes using withOrgPath (#70449)

commit 4d124bd
Author: Evan Hicks <evanh@users.noreply.github.com>
Date:   Wed May 8 11:19:07 2024 -0400

    fix: Add a metric for non-success Snuba requests (#70452)

    Tracking this on the Sentry side allows alerts to be created that are
    separate
    from the Snuba API itself, in case the API is in a broken state and
    can't
    accurately report what is happening.

commit 11a80a3
Author: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
Date:   Wed May 8 10:36:32 2024 -0400

    feat(cache): add average transaction duration to sample sidebar (#70445)

    New metric readout for avg transaction duration in sample sidebar
    <img width="746" alt="image"
    src="https://github.com/getsentry/sentry/assets/44422760/35146610-5c41-4e12-917e-e58310da84a9">

commit eb4de59
Author: Ash <0Calories@users.noreply.github.com>
Date:   Wed May 8 10:25:30 2024 -0400

    feat(perf): Add backend referrers for span summary and span metrics (#70466)

    Adds backend referrers to be used on the new span summary and span
    metrics pages

    Relevant PR: #69159

commit d3d6bdf
Author: Mark Story <mark@mark-story.com>
Date:   Wed May 8 10:20:37 2024 -0400

    chore(actor) Remove Actor model from django state (#70439)

    Remove the Actor model from django state.

    Refs HC-1183

commit 5bff933
Author: anthony sottile <103459774+asottile-sentry@users.noreply.github.com>
Date:   Wed May 8 08:34:04 2024 -0400

    ref: remove unused partition parameter from buffer (#70441)

    <!-- Describe your PR here. -->

commit 7512ed6
Author: Yagiz Nizipli <yagiz@nizipli.com>
Date:   Wed May 8 07:29:26 2024 -0400

    perf: use orjson in all middlewares (#70456)

commit 4623b5d
Author: Tony Xiao <txiao@sentry.io>
Date:   Wed May 8 06:07:45 2024 -0400

    fix(trace-explorer): Date range narrowing condition is backwards (#70496)

    This was changing the end timestamp to be too narrow and missing some
    spans.

commit fc8b666
Author: Ryan Hiebert <ryan@ryanhiebert.com>
Date:   Wed May 8 02:33:09 2024 -0500

    Use last forwarded IP (#68884)

    Well-behaved forwarders will append the IP they're forwarding for to an
    existing list. In the most typical case, this means that only the last
    one is trustworthy from a spoofing request-maker.

    And all this is assuming that the proxy itself is trusted.

    https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#security_and_privacy_concerns

    <!-- Describe your PR here. -->

    <!--

      Sentry employees and contractors can delete or ignore the following.

    -->

    ### Legal Boilerplate

    Look, I get it. The entity doing business as "Sentry" was incorporated
    in the State of Delaware in 2015 as Functional Software, Inc. and is
    gonna need some rights from me in order to utilize my contributions in
    this here PR. So here's the deal: I retain all rights, title and
    interest in and to my contributions, and by keeping this boilerplate
    intact I confirm that Sentry can use, modify, copy, and redistribute my
    contributions, under Sentry's choice of terms.

commit 01160aa
Author: Katie Byers <katie.byers@sentry.io>
Date:   Tue May 7 16:41:32 2024 -0700

    ref(seer-grouping): Switch to using `hash` and `parent_hash` from `group_hash` and `parent_group_hash` (#70383)

    As we've been thinking about the switch from sending and receiving group ids when communicating with Seer to doing so with hashes, all along we've been talking about those hashes as "group hashes." In truth, though, hash values are based on the data in a particular event (not the group overall), and indeed, that's how we're using them in Seer - pairing up hashes not with what group they're in but which event data they represent. There _is_ a pairing of groups and hashes - on the Sentry side, in the form of the `GroupHash` table - but entries from that table aren't what we're using with Seer. With Seer, we only care about the "hash" part of `GroupHash`.

    So, both for accuracy and so as to be able to differentiate in Seer-related Sentry code between hashes (hex strings) and grouphashes (association table records), we're switching from using `group_hash` and `parent_group_hash` to using `hash` and `parent_hash`.

    This PR makes the change on the Sentry side. Fortunately, nothing in Seer is yet relying on hashes, so as long as we wait for this to go live, we can then add hash support on the seer side using the new names from the get-go.

commit 173b690
Author: Raj Joshi <raj.joshi@sentry.io>
Date:   Tue May 7 16:32:23 2024 -0700

    fix(chartcuterie): Added Visual Map Field for Endpoint Regression (#70477)

    There is mismatch in the way we build the EChart Options object in our
    FE code and how Chartcuterie handles it.

    In our FE
    [code,](https://github.com/getsentry/sentry/blob/master/static/app/components/events/eventStatisticalDetector/breakpointChartOptions.tsx#L89-L104),
    we wrap the visualMap object in an extra option, which allows us to
    maintain the hierarchy for styling. However, Chartcuterie cannot handle
    the wrapped object, so when we pass the service the options, we unwrap
    it.

    I also created a modifier option to modify chart options specifically
    for slack and removed the icon from the legend icon from there.

    ![example2](https://github.com/getsentry/sentry/assets/33237075/2ecdf0db-3abd-4245-a426-b371c6a2fd98)

commit e624ee9
Author: Matt Duncan <14761+mrduncan@users.noreply.github.com>
Date:   Tue May 7 16:17:45 2024 -0700

    chore(issues): Opt in already passing issues files to stronger typing (#69828)

    `sentry.issues.*` and `test.sentry.issues.*` are not close to passing
    but in the mean time we can get incremental benefits and prevent
    regressions by opting in modules which are already passing.

    #69374 includes a bit more details and outlines additional fixes we can
    follow up this change with.

commit dbf524c
Author: Seiji Chew <67301797+schew2381@users.noreply.github.com>
Date:   Tue May 7 15:58:22 2024 -0700

    fix(ui): Revert to using project release commmit API (#70485)

    In #63860 we switched from a
    class component to a FC and also switched from hitting
    `ProjectReleaseCommitsEndpoint` to `OrganizationReleaseCommitsEndpoint`.

    The latter doesn't respect repo name or id in the query param, which
    makes it so selecting a repo in the dropdown will show you commits from
    other repos (and also messes up pagination as well, not showing you all
    commits).

    Related to #70411

commit 0b96de4
Author: Dan Fuller <dfuller@sentry.io>
Date:   Tue May 7 15:53:42 2024 -0700

    chore(crons): Rename badly named api file (#70480)

    I accidentally pluralised this when creating it, just fixing.

commit 2489a4a
Author: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
Date:   Tue May 7 18:19:21 2024 -0400

    fix(cache): update docs link (#70475)

    The link to the docs will be plural

commit 7ea571e
Author: Katie Byers <katie.byers@sentry.io>
Date:   Tue May 7 15:17:08 2024 -0700

    chore(events): Move `PLACEHOLDER_EVENT_TITLES` to a neutral location (#70470)

    I tried to use `PLACEHOLDER_EVENT_TITLES` in an upcoming PR, and landed in a circular dependency with `event_manager.py`, where it currently lives. This moves it to `constants.py`, which solves the problem.

commit a97dccf
Author: Michael Sun <55160142+MichaelSun48@users.noreply.github.com>
Date:   Tue May 7 18:11:47 2024 -0400

    chore(issue-stream): Register feature flag for upcoming changes to issues stream events graph (#70471)

    [Project details](#69691)

    * [SOA PR
    ](getsentry/sentry-options-automator#1378)
    * ~~getSentry PR~~ (No longer necessary)

commit fadd0b5
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Tue May 7 18:00:45 2024 -0400

    fix(ui): Use new illustration for tracing keyboard shortcuts (#70474)

    <img alt="clipboard.png" width="665"
    src="https://i.imgur.com/h8p3E1H.png" />

    Looks like the rest of the product now

commit 277f026
Author: Scott Cooper <scttcper@gmail.com>
Date:   Tue May 7 14:54:57 2024 -0700

    feat(metrics): Add metricSecond to allowed category (#70442)

commit ef6c79a
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Tue May 7 17:50:52 2024 -0400

    Revert "ref(crons): Normalize crons incident issues (#70289)" (#70469)

    This reverts commit 9d56889.

commit 45d2d2e
Author: Cathy Teng <70817427+cathteng@users.noreply.github.com>
Date:   Tue May 7 14:46:55 2024 -0700

    feat(slack): EA :white_circle: for actions by adding to issue alert threads FF (#70468)

commit 80ed536
Author: Tony Xiao <txiao@sentry.io>
Date:   Tue May 7 17:25:18 2024 -0400

    fix(trace-explorer): Breakdown by project and sdk (#70463)

    For full stack projects that use a single project for all the data, we
    need to break it down further using sdk.

commit 874db7e
Author: Katie Byers <katie.byers@sentry.io>
Date:   Tue May 7 14:24:19 2024 -0700

    ref(seer-grouping): Send group hash to Seer (#70244)

    This adds the group hash to the outgoing payload sent to Seer. Once this is merged, it will be safe to remove the Seer logic handling an incoming group id, since we'll now be able to rely on the group hash being present.

    Taken together, this PR and the ones leading up to it (links below) mean will mean that on the Sentry side we'll be sending in the request and handling in the response both hash and group id (and transforming hashes in the response into group ids if hashes are all that's sent). Once Seer is adjusted to only deal in hashes, we can then remove the sending and handling of group ids from Sentry and the transition will be complete.

    Other PRs which are part of the group-id-to-group-hash switch:
    - #70005, #70236, and #70237 - various small fixes and tweaks
    - #70070 and #70238 - updates to associated types
    - #70240 - automatic conversion from hash to group id when handling Seer similar group data

commit 78d2f28
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Tue May 7 17:23:38 2024 -0400

    ref(routes): Prioritize customer domains route (#70451)

    In a936fb0 we introduced a new
    withOrgPath prop to the Route component. This caused the route to have
    the same behaviour as doing

    ```tsx
    <Framgnet>
      {USING_CUSTOMER_DOMAIN && (
        <Route
          path="/some-path/"
          component={withDomainRequired(make(() => import('sentry/views/someView')))}
        />
      )}
      <Route
        path="/organizations/:orgId/some-path/"
        component={withDomainRedirect(make(() => import('sentry/views/someView')))}
      />
    </Framgnet>
    ```

    However the introduced logic generated the routes in the opposite order,
    where the org slug version would get priority.

    For some future changes this becomes important so I am bringing back
    this matching behaviour

commit 3a7e9b3
Author: Evan Purkhiser <evanpurkhiser@gmail.com>
Date:   Tue May 7 17:23:31 2024 -0400

    fix(crons): Teams -> Owners (#70464)

commit 2dbccba
Author: anthony sottile <103459774+asottile-sentry@users.noreply.github.com>
Date:   Tue May 7 17:13:26 2024 -0400

    ref: upgrade djangorestframework-stubs (#70461)

    upgrading this has no differences in ignored mypy errors

    <!-- Describe your PR here. -->

commit 0c8f916
Author: Mark Story <mark@mark-story.com>
Date:   Tue May 7 16:49:43 2024 -0400

    chore(database) Drop tables for project and team avatar (#68616)

    These tables no longer have django models, and can be deleted.

commit 4f71b8e
Author: Leander Rodrigues <leander.rodrigues@sentry.io>
Date:   Tue May 7 16:39:00 2024 -0400

    feat(highlights): Variety of fixes/changes to highlights work (#70355)

    This PR makes a variety of changes to the highlights/tags areas:
    - [x] Add tests for analytics and user friendly names

    - [x] Add search to the highlights modal

    ![image](https://github.com/getsentry/sentry/assets/35509934/75beb7c9-90c3-40a3-84d9-f4b585297de5)

    - [x] Allow `replayId`, `transaction` and tags with URLs to be clickable

    ![image](https://github.com/getsentry/sentry/assets/35509934/bc0f4ac6-a2c2-496a-bcfa-6e14c811d151)

    - [x] Adds a feedback button on Highlight section

    ![image](https://github.com/getsentry/sentry/assets/35509934/064edd41-1a83-48a9-ad12-b0de8102db4c)

    **todo**
    - [x] Add tests for search
    - [x] Add tests for new tag links
    - [x] Add screenshots of changes

commit aa40406
Author: Snigdha Sharma <snigdha.sharma@sentry.io>
Date:   Tue May 7 13:36:54 2024 -0700

    fix(escalating-issues): Cleanup query for escalating forecasts (#70443)

    Cleans up some cruft left behind by feature flags and pushes the
    escalation check out of the query to help with timeouts.

    Fixes SENTRY-18AS

commit 81d3fef
Author: Seiji Chew <67301797+schew2381@users.noreply.github.com>
Date:   Tue May 7 13:32:10 2024 -0700

    nit: Remove authenticators from state (#70365)

    Remove `authenticators` from state because we only set it once
@github-actions github-actions bot locked and limited conversation to collaborators May 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants