Revamp with latest driver, bot account support & more #43
Conversation
- Use the latest release of mattermost-server and switch to newer Go APIs - Use bot accounts / token instead of user account - Cleaner and more structured code - Don't panic when connection closes unexpectedly (when mattermost server dies or bot get disconnected ) - No global variables - Use env vars or .env file for configuration - Level based logging - Do not wait for one message to be handled before moving to next - Example to reply in an existing thread
There was a problem hiding this comment.
Great PR @IceWreck ! I have added myself and Martin as reviewers.
I've left a few comments (mainly in the README file and the db seeding script that creates initial users) and tested locally the PR and confirm it works as expected.
Thanks for the PR again!
| failCount := 0 | ||
| for { | ||
| app.mattermostWebSocketClient, err = model.NewWebSocketClient4( | ||
| fmt.Sprintf("ws://%s", app.config.mattermostServer.Host+app.config.mattermostServer.Path), |
There was a problem hiding this comment.
Maybe it is a better best practice to use wss (Web Socket Secure) by default? https://www.rfc-editor.org/rfc/rfc6455#page-55
There was a problem hiding this comment.
Someone doing bot development with the docker-compose in this repo won't have SSL on their local instance.
There was a problem hiding this comment.
We could parse the Mattermost server URL and determine whether we are using http or https, and adjust the websocket protocol accordingly
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
hanzei
added
2: Dev Review
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
Summary
BTW, is there a reason that latest tagged version of mattermost-server on go.dev is 6.7.X (from afew months ago) instead of Mattermost 7.1 (which is latest according to github releases) ?
Ticket Link
Solves #30 and more