Mattermost-server: Improve output of mattermost restore channel command

[mattermod]

Repro steps:

1. Run mattermost channel restore command on any archived channel

Observed: Command exits and there is no output indicating success
Expected: No unrelated errors and some output indicating success

---

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-19564

[promulo]

Hello, I was wondering if this would be a good first issue to work on. Does it make sense for a newcomer to take it? :slightly_smiling_face:

[amyblais]

@grundleborg Would you know?

[grundleborg]

Hi @promulo - I think it would be a reasonably simple ticket, although if it's your first ever one, you might want to try one with the Difficulty: Easy label first.

[promulo]

Hey @amyblais @grundleborg thanks for replying. In that case I'll have a look at the ones with that label. :slightly_smiling_face:

[saneletm]

Can I please look into this issue?

[saneletm]

I would like to wrap up the fix for this issue. Printing the success log when mattermost restore channel is called is trivial.

I am however having a hard time reproducing the error for Elasticsearch being stopped already is output. I am running on MacBook Pro 15 inch running version 10.15.1, and my mattermost fork is up to date. I am basically not seeing any errors on my box when I run the restore command.

On another note... why is it that the mattermost commands ouput is printed, instead of logged using the logger. They kind of look lost withing the command log.

Sample from running: mattermost team list:

`
....
s":1575285901.3405929,"caller":"plugin/health_check.go:32","msg":"Enabling plugin health check job",
"interval_s":30}
{"level":"debug","ts":1575285901.34065,"caller":"plugin/health_check.go:50","msg":"Plugin health check job starting."}
{"level":"info","ts":1575285901.3406742,"caller":"app/server.go:217","msg":"Current version is 5.18.0 (///)"}
{"level":"info","ts":1575285901.340747,"caller":"app/server.go:218","msg":"Enterprise Enabled: "}
{"level":"info","ts":1575285901.340802,"caller":"app/server.go:221","msg":"Printing current working","directory":"/User
s/sanelemahlalela/go/src/github.com/mattermost/mattermost-server"}
{"level":"info","ts":1575285901.340838,"caller":"app/server.go:222","msg":"Loaded config","source":"file:///Users/sanel
emahlalela/go/src/github.com/mattermost/mattermost-server/config/config.json"}
{"level":"info","ts":1575285901.355582,"caller":"sqlstore/post_store.go:1365","msg":"Post.Message has size restrictions
","max_characters":16383,"max_bytes":65535}
{"level":"debug","ts":1575285901.4133298,"caller":"jobs/schedulers.go:30","msg":"Initialising schedulers."}

mysampleteam

sanelethu

teamchw

{"level":"info","ts":1575285901.4391801,"caller":"app/server.go:346","msg":"Stopping Server..."}
{"level":"info","ts":1575285901.439265,"caller":"app/web_hub.go:125","msg":"stopping websocket hub connections"}
{"level":"warn","ts":1575285901.439287,"caller":"app/web_hub.go:130","msg":"We appear to have already sent the stop che
cking for deadlocks command"}
....
`

NOTE: I added empty lines between the team names to force the names to be in new lines.

[saneletm]

@jwilander would you help me reproduce the ElasticSearch Error?

[jwilander]

@saneletm it looks like that error only occurs if you're running with the enterprise bits included so I'd suggest not worrying about that and we can create an internal bug ticket to deal with that portion of this ticket

[saneletm]

@jwilander is there anything special about the channel restore command, which makes it more important to log success msg? I am asking because I can see other commands/sub-commands that do not log/print success msgs, including channel archive command

[jwilander]

Nothing special about this one command. Maybe @mgdelacroix can chime in, but IMO we should be printing success messages for all those command

[mgdelacroix]

I agree. As a rule, I think it is good to print some information if the operation succeeds, so the user has something else on top of the exit code to check that the command worked.

For this specific case, I'd print something like Channel CHANNEL_NAME restored successfully.

[saneletm]

Agreed and very easy to add. I would just have a hard time creating a PR for the fix on that command when I can see a lot of commands are lacking the same behavior. Of cause I can be convinced otherwise.

Since it seems like this is an issue only on success msgs, I think there would be an easy way
to get a general solution that doesn't touch too many files. Would we be open to this?

I also see two types of output from commands: prints, and logs.
Example channel list:
`
....
{"level":"debug","ts":1575285901.4133298,"caller":"jobs/schedulers.go:30","msg":"Initialising schedulers."}

mysampleteam

sanelethu

teamchw

{"level":"info","ts":1575285901.4391801,"caller":"app/server.go:346","msg":"Stopping Server..."}
.....
`

Example user delete:
`
...
s","max_characters":16383,"max_bytes":65535}
{"level":"debug","ts":1575578207.349298,"caller":"jobs/schedulers.go:30","msg":"Initialising schedulers."}
Have you performed a database backup? (YES/NO):
YES
Are you sure you want to permanently delete the specified users? (YES/NO):

{"level":"warn","ts":1575578281.033192,"caller":"app/user.go:1439","msg":"Attempting to permanently delete account","user_id":"dtcae9fnoin7iy4ys4g6tbft6e","user_email":"[email protected]"}
{"level":"warn","ts":1575578281.14166,"caller":"app/user.go:1540","msg":"Permanently deleted account","user_email":"[email protected]","user_id":"dtcae9fnoin7iy4ys4g6tbft6e"}
{"level":"info","ts":1575578281.14174,"caller":"app/server.go:346","msg":"Stopping Server..."}
....
`
Would it be desirable for these to be standardized (convert the prints to logs... they kind of look better)?

[jwilander]

I think it would be great if we could hide the logs and only show the prints unless a --verbose flag is provided. A lot of the time the log are just noise (but may be useful in some cases)

[jasonblais]

@saneletm touching base to make sure you have the info you need to proceed. If any questions, don't hesitate to let us know! Happy to clarify anything.

Really appreciate your help with this by the way :)

[saneletm]

@jasonblais I got wrapped up in all sorts of other tasks that I had. I know what I would like to do with this ticket, but wanted to investigate its feasibility. I basically would like to have a default success msg for all the commands, and also by default turn off the logging. I think I can put in some time investigating today. I apologize for the delay

[jasonblais]

Thanks @saneletm! Totally no worries, appreciate your interest on this ticket. Hope you're having a wonderful holiday season as well :) 🎄

[saneletm]

@jasonblais I have been working on my move to Berlin from Swaziland and it has taken more time and energy than I thought it was. I am in Berlin now and trying to get settled. I would still like to look into this issue but afraid I might not be able to make time for it this month. Can I release this ticket so that someone else can pick it up... if no one picks it up and I have some bandwidth, I'll pick it up once I get settled? Please let me know what you think

[jasonblais]

@saneletm certainly, that works. I've opened it up for grabs. And if someone has by then picked this ticket up, and you're interested to help contribute, we'd love to have your help with another help wanted ticket.

And that's a big move! I hope everything goes well as you settle in to Berlin!

[vesari]

I'd like to work on this if I may?

[jwilander]

Go for it @vesari ! Let us know if you have questions