geigerzaehler / beets-alternatives

Beets plugin to manage external files
MIT License
91 stars 21 forks source link

Crashes when the destination file exists, but wasn't created by this plugin #52

Open wisp3rwind opened 4 years ago

wisp3rwind commented 4 years ago

As discussed in #48, beets-alternatives crashes with a traceback if for some reason (most likely a bug in the plugin itself, or maybe due to a crashed alt update which wasn't committed to the database) the destination file/link already exists, but wasn't created by the plugin itself. This came up in the review comment https://github.com/geigerzaehler/beets-alternatives/pull/48#discussion_r396102264 with respect to SymlinkViews, but probably equally affects External and ExternalConvert collections. Possible paths forward:

1) Keep everything as-is and crash, since this plugin always works under the assumption that the destination directory is exclusively managed by itself. 2) Same as above, but log a warning instead and skip the affected item (or abort cleanly?). Minimum viable option IMO, any kind of crash has the potential to disrupt consistency between filesystem and beets' database. 3) Same as above, but overwrite without asking. 4) Query the user whether to replace the existing files. Probably not worth the effort and complexity in light of 1., see also @geigerzaehler's comments in the linked discussion in https://github.com/geigerzaehler/beets-alternatives/pull/48#discussion_r396102264.

geigerzaehler commented 4 years ago

I’d go with 1) over 2). The main reason is that under these unexpected we want to notify the user early and visibly. We also want to avoid doing more work and potentially messing up things. As you rightfully pointed out we would need to ensure that the consistency is not worse than with 2).

However, I leave the decision between 1) and 2) up to you, @wisp3rwind.