Skip to content

chore(slack): Use Slack SDK Client in validate_channel_id#73659

Merged
iamrajjoshi merged 5 commits into
masterfrom
raj/mes-ref/slack/sdk-2.22
Jul 2, 2024
Merged

chore(slack): Use Slack SDK Client in validate_channel_id#73659
iamrajjoshi merged 5 commits into
masterfrom
raj/mes-ref/slack/sdk-2.22

Conversation

@iamrajjoshi

Copy link
Copy Markdown
Collaborator

Update validate_channel_id to use to Slack SDK. Wrote Tests and Tested Locally. Addresses SDK-2.22

@iamrajjoshi iamrajjoshi requested a review from a team July 2, 2024 15:31
@iamrajjoshi iamrajjoshi self-assigned this Jul 2, 2024
@iamrajjoshi iamrajjoshi requested review from a team as code owners July 2, 2024 15:31
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 2, 2024
@codecov

codecov Bot commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 71.42857% with 8 lines in your changes missing coverage. Please review.

Project coverage is 78.04%. Comparing base (d724281) to head (b12825d).
Report is 40 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #73659   +/-   ##
=======================================
  Coverage   78.03%   78.04%           
=======================================
  Files        6640     6642    +2     
  Lines      297033   297083   +50     
  Branches    51152    51125   -27     
=======================================
+ Hits       231797   231848   +51     
  Misses      58961    58961           
+ Partials     6275     6274    -1     
Files Coverage Δ
src/sentry/options/defaults.py 100.00% <100.00%> (ø)
src/sentry/integrations/slack/utils/channel.py 80.60% <69.23%> (-0.57%) ⬇️

... and 48 files with indirect coverage changes

"integration_id": integration_id,
"channel_name": name,
"input_channel_id": input_channel_id,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Are we trying to follow a convention of declaring logger_params as a local variable? I don't think it's necessary unless the same params are logged in multiple places.

if e.text == "channel_not_found":
raise ValidationError("Channel not found. Invalid ID provided.")
_logger.info("rule.slack.conversation_info_failed", extra={"error": str(e)})
raise IntegrationError("Could not retrieve Slack channel information.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All four of these raise statements should have from e at the end.

@iamrajjoshi iamrajjoshi merged commit 39ded00 into master Jul 2, 2024
@iamrajjoshi iamrajjoshi deleted the raj/mes-ref/slack/sdk-2.22 branch July 2, 2024 18:30
@sentry

sentry Bot commented Jul 10, 2024

Copy link
Copy Markdown
Contributor

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 25, 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.

2 participants