slack: use fileuploadv2 for uploading#129
Conversation
asdfzdfj
force-pushed
the
master
branch
2 times, most recently
from
c4fe760 to
08b08ce
Compare
|
Looks good and thanks a lot for this contribution! Could you still add a changelog entry please? In general we do not have a maintainer of the Slack part of the bridge right now. Would you be willing to help out with that? |
changelog added as requested
unfortunately I don't know much Go, and only sorta showed up here due to workplace related issues, so I don't think I'll be able to dedicate effort for maintaining the slack bridge here that said, I might help out a bit with the slack bridge in the future, at least to keep it working in my workplace if nothing else (and from the looks of it, it seems to rely on the increasingly deprecated and discontinued slack api functions for its work, some upgrades is certainly needed) |
|
Welcome to the club. It seems like all the people currently involved are not much experienced with Golang. But ok, if we run into Slack specific issues we will mention you and maybe you can at least help us test it as I think no one else is a Slack user. Thanks! |
| } | ||
| if res.ID != "" { | ||
| // UploadFileV2 doesn't return the full slack.File info like the previous UploadFile, so query it separately | ||
| sfi, _, _, err := b.sc.GetFileInfo(res.ID, 0, 1) |
There was a problem hiding this comment.
This should be done in the background, otherwise it may block the main thread.
| initialComment += fmt.Sprintf(" with comment: %s", fi.Comment) | ||
| } | ||
| res, err := b.sc.UploadFile(slack.FileUploadParameters{ | ||
| res, err := b.sc.UploadFileV2(slack.UploadFileV2Parameters{ |
There was a problem hiding this comment.
This should be done in the background, otherwise it may block the main thread.
|
Looks good if it's working as intended! "minor" nitpick: network queries should be performed in their own goroutine so they don't block the main thread. From what i can read, it looks like all file attachments are uploaded in a single message. But the returned I'm not sure what the consequence of not providing the generated ID back to the gateway would be. Maybe that impacts message replies/edits? This is still an area of code/architecture we haven't fully investigated and documented. |
3 participants
simple patch to replace deprecated and now disabled
UploadFilewithUploadFileV2, which already automates the new file upload steps as described in the slack docsonly did a few scuffed tests with telegram <-> slack but it seems to be working
should fix #33