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

fix(slack): channel api deprecated #19446

Merged

Conversation

dcshiman
Copy link
Contributor

Slack api https://slack.com/api/channels.* is deprecated see:
Deprecating early methods in favor of the Conversations API
All new slack apps created after June 10th, 2020 will give error, which
creates issue for on premise sentry.

This commit fixes the issue by adding slack.legacy-app=(true/false) to sentry.yml
config so that it continues support for legacy slack apps while new apps
can toggle this option for new slack API support

Resolves: #7897

Slack api https://slack.com/api/channels.* is deprecated see:
https://api.slack.com/changelog/2020-01-deprecating-antecedents-to-the-conversations-api
All new slack apps created after June 10th, 2020 will give error, which
creates issue for on premise sentry.

This commit fixes the issue by adding `slack.legacy-app=(true/false)`
config so that it continues support for legacy slack apps while new apps
can toggle this option for new slack API support

Resolves: getsentry#7897
@hgajjar
Copy link

hgajjar commented Jun 19, 2020

This issue is preventing us from using Sentry effectively, as we can't get slack notifications.
Could someone from core team please review this PR, help to fix the failing builds and get it merged?

@benvinegar benvinegar requested a review from scefali June 19, 2020 06:22
@scefali
Copy link
Contributor

scefali commented Jun 19, 2020

@dcshiman Thanks for making this PR! This change definitely needed to be made. This looks good to me but I will test it out locally tomorrow to make sure it works as expected.

@dcshiman
Copy link
Contributor Author

@scefali thanks, I forked from 10.0.0 initially as we run this version on our on premise. However when i rebased onto master i saw slackV2 implemented with still using the old APIs. I guess its for v20.0.0 for which i am unable to start up the development environment on ubuntu so couldn't run a manual test but successfully tested on 10.0.0

Copy link
Contributor

@scefali scefali left a comment

Choose a reason for hiding this comment

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

Tested on my machine, looks good!

@scefali scefali merged commit cc9f7d1 into getsentry:master Jun 19, 2020
@hgajjar
Copy link

hgajjar commented Jun 26, 2020

Thank you for merging this @scefali , when can we expect a release with this fix?

@BYK
Copy link
Member

BYK commented Jun 26, 2020

Hi @hgajjar, all commits that land on master get available for on-premise as soon as the docker images are built. This fix seems to be out: https://hub.docker.com/r/getsentry/sentry/tags?page=1&name=cc9f7d1

If you are looking for a tagged release, looks like this fix missed the train for our 20.6.0 release so you can expect it to be in 20.7.0 (releasing on July 15th) or maybe in a 20.6.1 patch release if we ever release one.

You can read more about our new release schedule at https://blog.sentry.io/2020/06/22/self-hosted-sentry-switching-to-calver/

@phyzical
Copy link

phyzical commented Jul 3, 2020

can confirm this as working on latest master of onpremise with the new integration process but requires this to be added to config.yml, i will open a pr to add this incase it was missed.

slack.legacy-app: False

@dcshiman
Copy link
Contributor Author

dcshiman commented Jul 3, 2020

can confirm this as working on latest master of onpremise with the new integration process but requires this to be added to config.yml, i will open a pr to add this incase it was missed.

slack.legacy-app: False
@phyzical slack.legacy-app: false needs to be added to the config, i missed it out.

shouldn't we include a doc's page for this ? so far I see only this post as a guide or is there docs which I am not aware of ?

@phyzical
Copy link

phyzical commented Jul 3, 2020

there is https://develop.sentry.dev, infact upon closer inspection it looks like we just need to fork to add a section for slack in the integrations section.

ill do that later tonight probably :)

Edit: done 👍

@caseyduquettesc
Copy link

If anyone is using 9.1.2 on-premise and building from source, I back-ported this change as a patch. Our Slack integration works now. I also threw in the list cursor change to scan large channel lists in pages.

https://gist.github.com/caseyduquettesc/22f9b7ba401adc480e8ca739c23c79b4

@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slack Integration: Inability to connect private channel
6 participants