Add --ignore-file-not-found option to ignore HTTP not found errors downloading file attachments from Slack #27
Conversation
The default will be as before, to raise an HTTPError But with a more meaningful log msg for the error The user can choose to ignore these, making them warnings For now that is hardcoded in this commit to always ignore The next step is to hook this up to a command line option Empirically, this state is reproducible by deleting a file from Slack **after** the export, and before running the Discord import.
The default is to raise an exception, this allows an override. Previously this was hardcoded True for testing.
Also add `--users-file` to the documented command line
|
Attn @waocats and @ajroberts0417, feel free to review. (Doesn't look like I can actually list you as Assignees. Perhaps I'd have to add you as Collaborators to the project for that?) |
|
Also you're welcome to add me as a contributor if you want--would be happy to help where I can (and where my schedule permits). |
|
Yeah, I tested it locally for both the file deletion after export case, and revoking the download token. But they were just manual tests. I keep pondering setting up automated tests, but it's tricky. Either I'd need to rely on actual external services (Slack and Discord), or I'd have to do a whole bunch of mocking. And that mocking is perhaps of limited value, b/c it won't tell me if something actually changes on Slack or Discord's side to break things. So for now I don't have any automated testing, but it's kind of bugging me. But I suspect maybe ultimately just not worth the effort for this. |
This is a replacement for #21 . Not all attachments are images, so I have decided to not substitute not found files with a fallback image.
Instead, provide the option to deal with the situation in a manner similar to #20, that concerns files deleted from Slack before doing the export. In that case, we simply log a warning and continue.
This is potentially a more severe situation, so we want to give the user the ability to investigate further. The default behavior therefore stays as before, we raise an HTTPError after the failed download via
raise_for_status(). But give a somewhat more meaningful error msg, including pointing out to the user that they can override this behavior (and achieve similar behavior to the deleted file case) via the command line option--ignore-file-not-found.I have identified and replicated two situations in which this scenario can occur: