silverstripe / silverstripe-assets

Silverstripe Assets component

BSD 3-Clause "New" or "Revised" License

9 stars 66 forks source link

FileMigrationTask doesn't highlight complication when it runs #285

Open maxime-rainville opened 5 years ago

maxime-rainville commented 5 years ago

The File Migration tasks doesn't warn you when it does things out of the ordinary. e.g.: If your SS3 filename is invalid in SS4, it will rename the file without giving you an obvious heads up.

Also when migrating thousands of files, it's not obvious if a dozen of them failed for some reason.

Acceptance critera

[ ] As a developer running the file migration task, I can clearly see which files did not get migrated as expected.
[ ] Only errors which are non-recoverable stop the migration (most exceptions are caught, noted in the visible console output, and migration continues)
[ ] Data loss is avoided, for example edge cases where original files are deleted without the migration of this file being actually successful

Notes

Applies to all tasks involved in file migration (e.g. incl. "tags to shortcode"). Basically, go through and implement defensive coding, minimising edge cases
Charlie has actually experienced data loss, so this isn't a theoretical request

maxime-rainville commented 5 years ago

For the specific example of renaming an invalid filename, we could just add an extra check that will spit out a log warning instead of a log info, if the filename we get back from FlysystemAssetStore is different from what we were expecting.

In term of giving a clear list of what went wrong, we could try adding an extra LogHandler that capture any output with "warning" severity or above.

dnsl48 commented 5 years ago

Ok, I'm confused a little bit. Are we talking about errors or warnings? What do we expect end users to do with this logs?

maxime-rainville commented 5 years ago

We're talking about errors and warnings. The issue is you might have thousand of entries in those logs with only a handful of failures. You could easily miss those failures unless you pay attention closely.

dnsl48 commented 5 years ago

If we got prefixes there that could be a simple matter of grep by ERROR or WARN messages. AFAIK that's how many people read apache and nginx logs for example.

maxime-rainville commented 5 years ago

Indeed ... grep is the command of the bash Gods. Sadly, many developers still struggle through the dark night, unaware of the exalted righteousness of grep. We can only wait until all developers turn towards the magnificent light of grep and we can all join together at the brotherhood of the grep.

Until such time, we must endeavour to alleviate the torment of those sad souls who have yet to rejoice at the alter of grep.

grep sees all and searches all ... filtering out the wicked lines and returning the worthy ones.

dnsl48 commented 5 years ago

any text editor has search functionality as well :) also, I believe grep is easier to use than sake and file migration tasks, so I'm not sure that's for gods only ;)