lita-slack icon indicating copy to clipboard operation
lita-slack copied to clipboard

Ability to send content as a file.

Open boone opened this issue 8 years ago • 5 comments

We often need to attach text dumps to our bot replies. This PR adds a send_file_content method to handle this. It uses the content argument of Slack's files.upload API call, which the docs suggest is the proper way to create a file from a "long message/paste".

This is my first Lita-related PR so feedback is appreciated.

boone avatar Mar 04 '16 14:03 boone

Not sure what's up with the Travis failure. The specs pass locally, and they are based on the specs for set_topic which work OK.

boone avatar Mar 04 '16 14:03 boone

Thanks for the PR and welcome to the Lita community! This is very useful functionality and I'd like to get it or something like it added to the adapter. I won't be able to merge this as is, though, for a few reasons:

  • A general abstraction for file uploads in Lita has not yet landed (see litaio/lita#89). Assuming something like that will eventually be in Lita, I'd rather have the Slack adapter use that API and expose it directly on the adapter.
  • A prerequisite to a good upload API is to be able to stream the contents of the file so that the entire thing doesn't need to be loaded into memory at once. Unfortunately, Faraday, the HTTP library that both Lita and lita-slack use, does not support streaming. It's one of my goals to eventually replace Faraday with the http gem, which does support streaming, but this hasn't happened yet.

I'll leave this PR open to track the desire for this feature, but I'm afraid it'll need to wait for the above points to be addressed first.

P.S. You can rebase onto the latest master and you'll see the spec failure locally—your commit is based on an older master.

jimmycuadra avatar Mar 04 '16 20:03 jimmycuadra

Is this going to be merged soon - it's going to be a great problem solver from a chatops point of view, and making certain functionalites easily accessible by our users.

pdaugavietis avatar May 29 '16 15:05 pdaugavietis

Nope, my previous comment is still the current situation.

jimmycuadra avatar May 30 '16 01:05 jimmycuadra

@jimmycuadra what's the status to merge this PR?

joseluis-fw avatar Jul 26 '18 18:07 joseluis-fw