-
Notifications
You must be signed in to change notification settings - Fork 2k
badfiles: added config/cli options to automatically take action on error/warning #6295
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
base: master
Are you sure you want to change the base?
Changes from all commits
7310532
40b0ab4
9c8d976
fd77422
8430910
9be0a64
431882b
66ebf68
6f77da4
7e23992
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,18 +168,78 @@ def on_import_task_start(self, task, session): | |
| if checks_failed: | ||
| task._badfiles_checks_failed = checks_failed | ||
|
|
||
| def handle_import_action(self, action, failure_type): | ||
| if action == "abort": | ||
| ui.print_( | ||
| f"{ui.colorize('text_warning', 'Aborting')}" | ||
| f" due to import_action_on_{failure_type} configuration" | ||
| ) | ||
|
Comment on lines
+173
to
+176
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should define a mapping action_name_by_action = {"abort": "Aborting", ...}and then deduplicate ui.print_(
f"{ui.colorize('text_warning', action_name_by_action[action])}"
f" due to import_action_on_{failure_type} configuration"
)
if action == "abort":
raise importer.ImportAbortError()
if action == "skip":
return importer.Action.SKIP
return None |
||
| raise importer.ImportAbortError() | ||
| elif action == "skip": | ||
| ui.print_( | ||
| f"{ui.colorize('text_warning', 'Skipping')}" | ||
| f" due to import_action_on_{failure_type} configuration" | ||
| ) | ||
| return importer.Action.SKIP | ||
| elif action == "continue": | ||
| ui.print_( | ||
| f"{ui.colorize('text_warning', 'Continuing')}" | ||
| f" due to import_action_on_{failure_type} configuration" | ||
| ) | ||
| return None | ||
| else: | ||
| ui.print_( | ||
| ui.colorize( | ||
| "text_warning", | ||
| f"Got invalid import_action_on_{failure_type}" | ||
| f" configuration: {action}", | ||
| ) | ||
| ) | ||
| ui.print_( | ||
| ui.colorize( | ||
| "text_warning", | ||
| f"import_action_on_{failure_type} should be one of:" | ||
| f" ask abort skip continue", | ||
| ) | ||
| ) | ||
| raise importer.ImportAbortError() | ||
|
Comment on lines
+190
to
+205
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should use |
||
|
|
||
| def on_import_task_before_choice(self, task, session): | ||
| if config["import"]["quiet"]: | ||
| return None | ||
|
|
||
| if hasattr(task, "_badfiles_checks_failed"): | ||
| warning_action = self.config["import_action_on_warning"].get("ask") | ||
| error_action = self.config["import_action_on_error"].get("ask") | ||
|
Comment on lines
+212
to
+213
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add default configuration inside self.config.add(
{
"import_action_on_warning": "ask",
...,
}
)See other plugins for examples. |
||
|
|
||
| ui.print_( | ||
| f"{colorize('text_warning', 'BAD')} one or more files failed checks:" | ||
| ) | ||
|
|
||
| found_warning = False | ||
| found_error = False | ||
| for error in task._badfiles_checks_failed: | ||
| for error_line in error: | ||
| if ( | ||
| "checker found 0 errors or warnings" | ||
| in error_line.lower() | ||
| ): | ||
| continue | ||
|
|
||
| if "warning" in error_line.lower(): | ||
| found_warning = True | ||
| if "error" in error_line.lower(): | ||
| found_error = True | ||
|
|
||
| ui.print_(error_line) | ||
|
|
||
| # Check for and handle automatic actions. | ||
| # Errors always take precedence over warnings. | ||
| if found_error and error_action != "ask": | ||
| return self.handle_import_action(error_action, "error") | ||
| elif found_warning and warning_action != "ask": | ||
| return self.handle_import_action(warning_action, "warning") | ||
|
|
||
| ui.print_() | ||
| ui.print_("What would you like to do?") | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -355,7 +355,6 @@ def _get_album_info( | |
| track_by_id: dict[str, TidalTrack], | ||
| artist_by_id: dict[str, TidalArtist], | ||
| ) -> AlbumInfo: | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be undone as it's unrelated. |
||
| track_infos: list[TrackInfo] = [] | ||
| for i, track_rel in enumerate( | ||
| album["relationships"]["items"]["data"], start=1 | ||
|
|
||
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.
Would you mind adding
Literaltypes here for clarity?