FEATURE: Attach backup log as upload (PR #13849)

Discourse automatically sends a private message after backup finished. The private message used to contain the log inline, but that is a problem if the log is very long because the length of the post will be over the maximum allowed length of a post.

GitHub

Why would happen here if upload is not persisted?

I think we should only warn in our logs if the warning can help developers debug the problem. Otherwise, adding to the logs is just going to add noise to the logs.

I think it is better to be more specific here where we test the name of the upload and maybe the file type?

Just curious, did we consider what would happen if the file is greater than SiteSetting.max_attachment_size_kb?

Validation will fail, upload will not persist and that case is handled outside this function call.

The PM will continue the error message why the upload did not persist.

I added upload error messages.

That is a good idea.

We can use the helpers here to capture the output instead of stubbing.

Since the description of the test states that it includes the upload error, I think we should assert for the post’s body instead of just checking a topic has been created.

I feel like the error message here is not going to be useful for the user. Perhaps we should just trancate the logs to a sane length should there be an upload failure?

I updated the PR to include the backup logs whenever it is possible.

The changes look great to me :+1:

I would probably add a mention that explains the logs were trimmed.

If you don’t need logfile, you could maybe use the short version?

        File.write(File.join(dir, filename), pretty_logs)