Skip to content

Discord proxy#113

Open
selfhoster1312 wants to merge 1 commit into
matterbridge-org:masterfrom
selfhoster1312:discord-proxy
Open

Discord proxy#113
selfhoster1312 wants to merge 1 commit into
matterbridge-org:masterfrom
selfhoster1312:discord-proxy

Conversation

@selfhoster1312

Copy link
Copy Markdown
Collaborator

This is the continuation of #109 and #48.

Apparently i couldn't push to @Wohlstand's branch on #109 so i had to open this PR here.

@selfhoster1312

Copy link
Copy Markdown
Collaborator Author

Ready for review/testing @poVoq @Wohlstand

Comment thread bridge/discord/handlers.go Outdated
Comment thread bridge/discord/handlers.go Outdated
helper.HandleDownloadData(b.Log, &rmsg, name, caption, "", data, b.General)
// TODO: does the discord API have a unique ID for the file?
// in the meantime, let's reuse the filename
err = b.AddAttachmentFromURL(&rmsg, attach.Filename, attach.Filename, caption, attach.URL)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should be processed in the background instead of blocking the main thread. This is wrong and we should merge #114 first and not edit file download at all here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

On my end I already merged that: Wohlstand@3043425

@Wohlstand

Copy link
Copy Markdown

I rebased this to the latest branch locally and tested on my end, it works for me. Additionally I applied the similar tweak to Telegram (just re-use of existing parameter and configuring bot instance with a custom client and proxied transomrt), otherwise the bridge was broken on my side, and I fixed it by adding the proxy here too. But, this tweak directly depends on this one.

@selfhoster1312

Copy link
Copy Markdown
Collaborator Author

I just rebased myself. Let me know if this branch works for you @Wohlstand and we can merge it.

Sorry in the rebase i lost your authorship information in the comment. If that's important to you, i can put it back somehow. Or you can open a new PR with your authorship :)

Comment thread bridge/discord/discord.go Outdated
@Wohlstand

Copy link
Copy Markdown

@selfhoster1312, it's just matching the state at my production build, and it works. 👍

@selfhoster1312

Copy link
Copy Markdown
Collaborator Author

@selfhoster1312, it's just matching the state at my production build, and it works. 👍

Great, could you maybe confirm if you shut down the proxy the success message is still printed? We should move it right after b.c.Open right?

@Wohlstand

Copy link
Copy Markdown

Great, could you maybe confirm if you shut down the proxy the success message is still printed? We should move it right after b.c.Open right?

Of course it gets printed, because it's just a dumb print command, and it's supposed to print the success connection report after the connection function return or at the callback that confirms the connection was successfull.

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.

2 participants