Add reflink option

[sampsyo]

This is a continuation of #2643, by @rubdos.

Here are a few outstanding questions:

• How shall we document the need to install the reflink module?
• What should happen when it's not installed? Should a friendly error message remind you to install it, or should we warn and fall back to copying?
• Do we really need the auto option, or should reflink: yes automatically fall back to copying? (At the moment, I can't think of a situation where I'd want to use yes, i.e., fail with an error when reflinking is impossible.)

[sampsyo]

Also, it looks like were getting two spurious (unrelated) Travis failures? Hmm…

[sampsyo]

Also, it looks like were getting two spurious (unrelated) Travis failures? Hmm…

[rubdos]

How shall we document the need to install the reflink module?

Just in the documentation, I'd think. No real need to put that somewhere else beyond the configuration option.

What should happen when it's not installed? Should a friendly error message remind you to install it, or should we warn and fall back to copying?

I'd say error out if it's not there, and reflink: yes/auto is set (so no error with reflink: no, otherwise, error out).

Do we really need the auto option, or should reflink: yes automatically fall back to copying? (At the moment, I can't think of a situation where I'd want to use yes, i.e., fail with an error when reflinking is impossible.)

Yes. The use case is as follows: some people will want their collection to be ALWAYS reflinked, and be warned thoroughly when it's not possible to link, while others want it to reflink when possible (when e.g. data is already on the same partition), and not when it's elsewhere (e.g. importing from a USB key from a friend).

[rubdos]

How shall we document the need to install the reflink module?

Just in the documentation, I'd think. No real need to put that somewhere else beyond the configuration option.

What should happen when it's not installed? Should a friendly error message remind you to install it, or should we warn and fall back to copying?

I'd say error out if it's not there, and reflink: yes/auto is set (so no error with reflink: no, otherwise, error out).

Do we really need the auto option, or should reflink: yes automatically fall back to copying? (At the moment, I can't think of a situation where I'd want to use yes, i.e., fail with an error when reflinking is impossible.)

Yes. The use case is as follows: some people will want their collection to be ALWAYS reflinked, and be warned thoroughly when it's not possible to link, while others want it to reflink when possible (when e.g. data is already on the same partition), and not when it's elsewhere (e.g. importing from a USB key from a friend).

[rubdos]

Also, it looks like were getting two spurious (unrelated) Travis failures? Hmm…

On first sight, doesn't look like reflink code, or is it? Feel free to give me a ping on IRC/gitter/whatever. :-)

[rubdos]

Also, it looks like were getting two spurious (unrelated) Travis failures? Hmm…

On first sight, doesn't look like reflink code, or is it? Feel free to give me a ping on IRC/gitter/whatever. :-)

[sampsyo]

OK, that all sounds good! Let's add a user-visible error message when the module isn't available.

Indeed, the Travis failures are incidental. In particular, I'm wrestling with a dependency suddenly becoming incompatible with old versions of Python 2.7 and Travis simultaneously deciding to revert to an old version of Python 2.7… 😢

[sampsyo]

OK, that all sounds good! Let's add a user-visible error message when the module isn't available.

Indeed, the Travis failures are incidental. In particular, I'm wrestling with a dependency suddenly becoming incompatible with old versions of Python 2.7 and Travis simultaneously deciding to revert to an old version of Python 2.7… 😢

[rubdos]

Good, let's get this merged. I rebased it on master, I'll now:

[...] add a user-visible error message when the module isn't available.

[rubdos]

Good, let's get this merged. I rebased it on master, I'll now:

[...] add a user-visible error message when the module isn't available.

[rubdos]

[...] add a user-visible error message when the module isn't available.

Okay, now I actually wonder what kind of error message this should be; should I throw an exception when the module cannot be imported, then catch that somewhere and print something nice? I tried reaching out over IRC, but didn't seem to get a reaction :-)

[rubdos]

[...] add a user-visible error message when the module isn't available.

Okay, now I actually wonder what kind of error message this should be; should I throw an exception when the module cannot be imported, then catch that somewhere and print something nice? I tried reaching out over IRC, but didn't seem to get a reaction :-)

[sampsyo]

Yeah, I think so! I'm not 100% sure about how the error should be exposed internally—perhaps a new exception just for this purpose?—but eventually, we have ui.UserError for displaying unrecoverable errors to the user.

[sampsyo]

Yeah, I think so! I'm not 100% sure about how the error should be exposed internally—perhaps a new exception just for this purpose?—but eventually, we have ui.UserError for displaying unrecoverable errors to the user.

[rubdos]

[...] add a user-visible error message when the module isn't available.

Okay, now I actually wonder what kind of error message this should be; should I throw an exception when the module cannot be imported, then catch that somewhere and print something nice? I tried reaching out over IRC, but didn't seem to get a reaction :-)

Yeah, I think so! I'm not 100% sure about how the error should be exposed internally—perhaps a new exception just for this purpose?—but eventually, we have ui.UserError for displaying unrecoverable errors to the user.

I suppose inherting ui.UserError would be appropriate here?

[rubdos]

[...] add a user-visible error message when the module isn't available.

Okay, now I actually wonder what kind of error message this should be; should I throw an exception when the module cannot be imported, then catch that somewhere and print something nice? I tried reaching out over IRC, but didn't seem to get a reaction :-)

Yeah, I think so! I'm not 100% sure about how the error should be exposed internally—perhaps a new exception just for this purpose?—but eventually, we have ui.UserError for displaying unrecoverable errors to the user.

I suppose inherting ui.UserError would be appropriate here?

[rubdos]

Hmm, beets.util has a HumanReadableException too, which one should I take? :'-)

[rubdos]

Hmm, beets.util has a HumanReadableException too, which one should I take? :'-)

[sampsyo]

It might be useful to see the evolving discussion in #2785, but I think raising a custom exception or a FilesystemError from the utility function itself is the right thing for util. Then, to actually show the problem to the user—if we want to stop the program altogether—higher levels in the stack should probably catch this at an opportune moment and raise a UserError.

Of course, if we wanted the same behavior as for trying to write to a read-only filesystem, for example, then just keeping it as a FilesystemError would be the right thing.

[sampsyo]

It might be useful to see the evolving discussion in #2785, but I think raising a custom exception or a FilesystemError from the utility function itself is the right thing for util. Then, to actually show the problem to the user—if we want to stop the program altogether—higher levels in the stack should probably catch this at an opportune moment and raise a UserError.

Of course, if we wanted the same behavior as for trying to write to a read-only filesystem, for example, then just keeping it as a FilesystemError would be the right thing.

[rubdos]

So, it's probably most useful to wait for #2785 to be resolved then.

[rubdos]

So, it's probably most useful to wait for #2785 to be resolved then.