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

Add new slack returner based on incoming webhooks #50879

Merged
merged 7 commits into from
Dec 26, 2018

Conversation

cdalvaro
Copy link
Contributor

@cdalvaro cdalvaro commented Dec 16, 2018

What does this PR do?

Add a new slack returner based on incoming webhooks

slack_webhook_returner

Tests written?

No

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@damon-atkins
Copy link
Contributor

What is the difference between salt/returners/slack_returner and this code?

@cdalvaro
Copy link
Contributor Author

What is the difference between salt/returners/slack_returner and this code?

slack_retuner does not work with Slack Incoming Webhooks, which is the recommended way to post messages in Slack rather than using the Legacy API Tokens

@damon-atkins
Copy link
Contributor

So can you keep them compatible? What else needs to be changed in salt to work with the new API?

@cdalvaro
Copy link
Contributor Author

cdalvaro commented Dec 17, 2018

So can you keep them compatible? What else needs to be changed in salt to work with the new API?

Do you mean, what changes are necessary to be made in slack_returner in order to make it compatible with the new API too?

If that is your question, I think that changes should be minimum. Just adding the parameter for the webhook url and based on whether a webhook is passed or an api_token choose the post method.

But this returner is also different in the message format.

@cdalvaro
Copy link
Contributor Author

Will this PR finally be approved? Or it is needed some change?

@thatch45 thatch45 merged commit 6bde57e into saltstack:develop Dec 26, 2018
@cdalvaro
Copy link
Contributor Author

Thank you very much, @thatch45 !

@thatch45
Copy link
Contributor

Sure thing, but I likely should not have merged this without tests. If you can get some tests in it would be greatly appreciated!

@cdalvaro cdalvaro deleted the returners/slack_webhook branch December 27, 2018 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants