-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
fix(slack): don't raise RuntimeError if unable to fetch property in slack request #67798
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure this doesn't break our async process where we try to get the team/channel id when the user only passes in team/channel name
@@ -115,20 +115,20 @@ def integration(self) -> RpcIntegration: | |||
return self._integration | |||
|
|||
@property | |||
def channel_id(self) -> str: | |||
def channel_id(self) -> str | None: | |||
return get_field_id(self.data, "channel") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we use _get_field_id_option
here as well, or no? Wonder why it's different
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we always expect a channel. i forgot to undo the type change
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
I'm waiting for this |
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Please merge this PR 🙏 |
Resolves #67679
We are currently raising a
RuntimeError
if thechannel_id
,team_id
, oruser_id
does not exist in an incoming Slack request. We useteam_id
anduser_id
here:sentry/src/sentry/integrations/slack/requests/base.py
Lines 82 to 92 in 121726d
And these values can be
None
, so we don't need to be raising aRuntimeError
:sentry/src/sentry/services/hybrid_cloud/integration/impl.py
Lines 457 to 464 in 121726d
NOTE: Not yet sure if it's okay for
channel_id
to beNone
. We can remove that part if need be.Replacing
get_field_id
which raises the error with_get_field_id_option
which can returnNone
should fix this.