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

Use chat.postMessage API for posting a message #60

Merged
merged 4 commits into from
Jul 4, 2019

Conversation

Chikashi-Kato
Copy link
Member

Add a logic to use chat.postMessage API to post a message to Slack if webhook_url is not set.
Now, webhook_url is deprecated and is not set in the interactive setup mode (--setup).
This is a backward compatible change and slacktee is still use and prioritize webhook_url if it's set.

This PR should fix issue #56.

slacktee.sh Outdated Show resolved Hide resolved
README.md Outdated
For more details about tokens, visit [Slack's API page](https://api.slack.com/).
In order to retrieve the bot token, you need to add a bot into your workspace through [Slack App Directory](https://cks-world.slack.com/apps/A0F7YS25R-bots).
Once you add a bot, you can find 'API Token' in the configuration page of the bot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I would word it in steps. coz its either or. so the comment can say:
You would need an authentication token for the bot. It could be generated in 2 ways

  1. (Preferred way) Follow steps listed in creating a Slack App.
    Next, create a bot user for your app and give the following 3 permissions to your app - chat:write:bot, files:write:user and bot. More information about the permission scopes can be found at permission scopes
  2. Add a bot into your workspace through Slack App Directory. You can now find 'API Token' in the configuration page of the bot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, great suggestion! I will update README!

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

@@ -22,8 +22,8 @@
# ----------
# Default Configuration
# ----------
webhook_url="" # Incoming Webhooks integration URL
token="" # The user's API authentication token, only used for file uploads or streaming
webhook_url="" # Incoming Webhooks integration URL. Old way to interact with Slack. (Deprecated)

Choose a reason for hiding this comment

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

Could we log a deprecation message to stderr if webhook_url is set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! Good idea! Let me implement it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented!

@@ -203,7 +203,8 @@ function send_message()
\"icon_url\": \"$icon_url\" $parseMode}"

post_result=$(curl -H "Authorization: Bearer $token" -H 'Content-type: application/json; charset=utf-8' -X POST -d "$json" https://slack.com/api/chat.postMessage 2> /dev/null)
if [ $? != 0 ]; then
post_ok="$(echo "$post_result" | awk 'match($0, /"ok":([^,}]+)/) {print substr($0, RSTART+5, RLENGTH-5)}')"

Choose a reason for hiding this comment

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

Starting to wonder if we are tying our hands in complexity with using only shell scripts here. I understand the goal is to keep slacktee as cross-platform as possible, but consider:

  1. python (well, at least python 2) is included in the majority of OSes by default (although with a few exceptions re: BSD. Do we have users running slacktee w/o python installed? Do we know?)
  2. We are using bash, not sh. Are we using any bash extensions that aren't cross-platform? I've no idea.

If we've got all this complex shell script, I just want to make sure we are truly gaining something (cross-platform) that is actually needed (do our users need this)? I'm afraid there's like, 2 or 3 people in the world that understand this script :)

Copy link
Contributor

Choose a reason for hiding this comment

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

We could potentially do a binary with go, that is built per distro. Any dependencies can be baked in (static binary).

Choose a reason for hiding this comment

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

Ah, I see now.

I would hesitate to use golang for this, just because it seems making a CLI tool in Python would be simpler, and more people could easily contribute.

But I do like the idea of installing missing deps (ie: python) w/ Ansible if missing. However, I think that might make integrating the script harder? right now you just drop the shell file on any system you want - idk how Ansible works, would it be that simple?

Copy link
Member Author

@Chikashi-Kato Chikashi-Kato Jul 3, 2019

Choose a reason for hiding this comment

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

I might be too stubborn but I would like to avoid using other languages in slacktee.
slacktee is intentionally written in the shell script for the simple installation and the better portability. I agree python is now available in majority of OSes by default, but some people are using slacktee in bare minimum environments (see #37 and #57) and I cannot imaging that they are happily installing python just for using slacktee.

With regard to point 2, I'm not sure if we are using non-cross-platfrom bash extensions. In my understanding, we are currently using [[ ]] which is not available in sh for using regex in the if condition. Some OS like alpine doesn't have bash by default and using sh might be better for the portability.
If we drop the support of conditional formatting, we may be able to make slacktee a sh script ;)

Choose a reason for hiding this comment

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

If we drop the support of conditional formatting, we may be able to make slacktee a sh script ;)

The opposite intention of my comment :P

To be more extreme, I wonder if there is a way to transpile a restrictive subset of some higher level language into a POSIX compliant sh script. Basically, I think it'd be awesome to have an easier language to develop in, but keep the awesome portability you want to uphold. An idea for another day :)

Copy link
Member Author

@Chikashi-Kato Chikashi-Kato Jul 3, 2019

Choose a reason for hiding this comment

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

To be clear, I am not opposing to implement slacktee functionalities in other languages.
If we start another project which is implementing slacktee in different languages (e.g. slacktee-python and slacktee-go), I would love to contribute!

@connorjclark
Copy link

lgtm. left one nit re: deprecation, and one comment for broader discussion on complexity.

@jimsmith
Copy link

jimsmith commented Jul 3, 2019

tbh this shell script has grown organically and should be massively simplified with say Ansible (IMHO)

@connorjclark
Copy link

Could you explain that a bit? AFAIU, Ansible is a tool for deployment configuration - do you think it'd be better for a script than something like Python? Not sure I understand :)

@Chikashi-Kato
Copy link
Member Author

@jimsmith I would like to understand your opinion more deeply.
I agree that this script has now so many features and the implementation of the script is complicated.
How can we simplify the implementation of this script with Ansible?

@jimsmith
Copy link

jimsmith commented Jul 3, 2019

Python example
https://www.accadius.com/send-message-slack-python-program/

Ansible example
https://docs.ansible.com/ansible/latest/modules/slack_module.html

https://www.linkedin.com/pulse/posting-slack-using-ansible-tariq-siddiqui/

I started off with this script years back but as it grew complexity I switched quickly to Ansible and for python lovers there is the above example.

No need to do complex condition statements :)

@connorjclark
Copy link

connorjclark commented Jul 3, 2019

Interesting! The Ansible approach seems great for when you know before-hand exactly the type of messages you want to send to Slack. If seems to offer a great way to configure message templates. But with slacktee, the use case seems a bit different. It's tacked onto the end of just about any command, and is used more adhoc in scripts/crons.

Or perhaps there's a way to create these ansible configs programmatically, and let them do the heavy lifting? But at that point, I wonder if just leveraging (for example) the Python Slack SDK would provide the same ease.

@Chikashi-Kato Chikashi-Kato merged commit 1fdf6c1 into master Jul 4, 2019
@Chikashi-Kato Chikashi-Kato deleted the 56-use-token-for-posting-messages branch July 4, 2019 02:13
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.

None yet

6 participants