New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MoveOperation bugfix for CLI overrides #36
Conversation
@@ -333,9 +335,6 @@ def process_artifacts(self, source_files, mapping): | |||
util.mkdirall(dest_file) | |||
dest_file = util.bytestring_path(dest_file) | |||
|
|||
# TODO: detect if beets was called with 'move' and override config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a legacy TODO
that is now resolved.
The beets.config
module provides the current config when referenced and not just the values in config.yml
. Fixing when the self.operation
is obtained deprecates this TODO.
@@ -305,10 +314,10 @@ def _list_files(self, startpath): | |||
for root, _dirs, files in os.walk(path): | |||
level = root.replace(path, "").count(os.sep) | |||
indent = self._indenter(level) | |||
log.debug("%s%s/", indent, os.path.basename(root)) | |||
log.debug(f"{indent}{os.path.basename(root)}/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With how the logger is passed around, %s
interpolation doesn't occur.
Context
As pointed out in #35, certain configuration scenarios can result in the
MoveOperation
selection isNone
. In that issue, @seanbreckenridge is right that this comes down to when the plugin defines (really, captures) the desired Operation (copy, move, etc.) that is occurring in the import.Previously, this was captured on initialization in the constructor for Filetote. However, due to the lifecycle of
beets
, this is called before theimport
command initializes/runs. As a side effect, any of the overriding CLI options passed along (relevant here,--move
or--copy
) would set/override the operation on the media file(s) but, since the operation was defined before this and not updated, the extra files that Filetote would manage would utilize a different operation than the media file.In addition, if the
config.yml
explicitly did not set an operation (meaning, all werefalse
) and if one were to rely on the CLI only, theMoveOperation
would result inNone
(the plugin initializes withNone
which matches theconfig.yml
but the operation is not updated once the import starts to grab the option provided in the CLI).Other notes
beets
prioritize the operation as the options are mutually exclusive. There are three layers to how this occurs:config.yml
, prioritizing in this order (importer.py
):move
copy
link
hardlink
reflink
--move
or--copy
):--move
and--copy
) are provided, it prioritizescopy
(ui/commands.py
)There are a few more nuances, but this covers the vast majority of cases.
Changes
This moves the setting of the operation used by Filetote to the
import_begin
event that is triggered at the start of the import process. This also updates/adds tests for operations provided via CLI.This also includes the suggested change from #34 which is, indeed, helpful for debugging.
Big thanks to @seanbreckenridge for running much of this down and documenting their journey in such detail!