From 7765fac818e733c2789fd3f975375440a50f5a00 Mon Sep 17 00:00:00 2001
From: Maximilian Moser <maximilian.moser@tuwien.ac.at>
Date: Tue, 13 Jul 2021 17:50:54 +0200
Subject: [PATCH] Make output more consistent

* use f-strings everywhere
* print error messages in yellow to stderr
---
 invenio_utilities_tuw/cli/drafts.py  | 20 ++++++++++++--------
 invenio_utilities_tuw/cli/files.py   | 17 +++++++++++------
 invenio_utilities_tuw/cli/records.py | 12 +++++++-----
 invenio_utilities_tuw/cli/users.py   |  8 ++++----
 4 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/invenio_utilities_tuw/cli/drafts.py b/invenio_utilities_tuw/cli/drafts.py
index 993c65c..8151a67 100644
--- a/invenio_utilities_tuw/cli/drafts.py
+++ b/invenio_utilities_tuw/cli/drafts.py
@@ -128,8 +128,8 @@ def create_draft(metadata_path, publish, user, owners, vanity_pid):
 
             if len(content) != len(file_names):
                 ignored = [basename(fn) for fn in content if not exists(fn)]
-                msg = "ignored in '{}': {}".format(deposit_files_path, ignored)
-                click.secho(msg, fg="red", err=True)
+                msg = f"ignored in '{deposit_files_path}': {ignored}"
+                click.secho(msg, fg="yellow", err=True)
 
         file_service.init_files(
             id_=recid, identity=identity, data=[{"key": fn} for fn in file_names]
@@ -280,8 +280,8 @@ def add_files(filepaths, pid, pid_type, user):
 
             if len(content) != len(file_names):
                 ignored = [basename(fn) for fn in content if not exists(fn)]
-                msg = "ignored in '{}': {}".format(file_path, ignored)
-                click.secho(msg, fg="red", err=True)
+                msg = f"ignored in '{file_path}': {ignored}"
+                click.secho(msg, fg="yellow", err=True)
 
             paths_ = [join(file_path, fn) for fn in file_names]
             paths.extend(paths_)
@@ -291,7 +291,9 @@ def add_files(filepaths, pid, pid_type, user):
 
     keys = [basename(fp) for fp in paths]
     if len(set(keys)) != len(keys):
-        click.secho("aborting: duplicates in file names detected", fg="red", err=True)
+        click.secho(
+            "aborting: duplicates in file names detected", fg="yellow", err=True
+        )
         sys.exit(1)
 
     file_service.init_files(
@@ -354,7 +356,7 @@ def list_files(pid, pid_type, user):
     for name, rec_file in draft.files.entries.items():
         fi = rec_file.file
         if fi:
-            click.secho("{}\t{}\t{}".format(name, fi.uri, fi.checksum), fg="green")
+            click.secho(f"{name}\t{fi.uri}\t{fi.checksum}", fg="green")
         else:
             click.secho(name, fg="red")
 
@@ -378,12 +380,14 @@ def verify_files(pid, pid_type):
             click.secho(name, fg="green")
 
         else:
-            click.secho("{}: failed checksum verification".format(name), fg="red")
+            click.secho(f"{name}: failed checksum verification", fg="yellow", err=True)
             num_errors += 1
 
     if num_errors > 0:
         click.secho(
-            "{} files failed the checksum verification".format(num_errors), fg="red"
+            f"{num_errors} files failed the checksum verification",
+            fg="yellow",
+            err=True,
         )
         sys.exit(1)
 
diff --git a/invenio_utilities_tuw/cli/files.py b/invenio_utilities_tuw/cli/files.py
index f85541b..138465a 100644
--- a/invenio_utilities_tuw/cli/files.py
+++ b/invenio_utilities_tuw/cli/files.py
@@ -98,7 +98,7 @@ def list_deleted_files(pid, pid_type):
     # delete the associated FileInstances, and remove files from disk
     for key in file_instances:
         for fi in file_instances[key]:
-            click.secho("{}\t{}".format(key, fi.uri), fg="green")
+            click.secho(f"{key}\t{fi.uri}", fg="green")
 
     db.session.commit()
 
@@ -141,7 +141,7 @@ def hard_delete_files(pid, pid_type):
                 storage = fi.storage()
                 fi.delete()
                 storage.delete()
-                click.secho("{}\t{}".format(key, fi.uri), fg="red")
+                click.secho(f"{key}\t{fi.uri}", fg="red")
 
             except Exception as error:
                 click.secho(
@@ -167,15 +167,15 @@ def list_orphan_files():
     for loc in Location.query.all():
         # we only know how to handle directories on the file system for now
         if os.path.isdir(loc.uri):
-            click.echo("location: {}".format(loc.name))
+            click.echo(f"location: {loc.name}")
         else:
             click.secho(
-                "warning: location '{}' is not a path: {}".format(loc.name, loc.uri),
+                f"warning: location '{loc.name}' is not a path: {loc.uri}",
                 fg="yellow",
             )
 
         for fp in get_orphaned_files(loc):
-            click.echo("  {}".format(fp))
+            click.echo(f"  {fp}")
 
 
 @orphans.command("clean")
@@ -188,6 +188,11 @@ def clean_files():
     for loc in Location.query.all():
         # we only know how to handle directories on the file system for now
         if not os.path.isdir(loc.uri):
+            click.secho(
+                f"don't know how to handle location '{loc.name}': skipping",
+                fg="yellow",
+                err=True,
+            )
             continue
 
         for fp in get_orphaned_files(loc):
@@ -196,4 +201,4 @@ def clean_files():
                 click.secho(fp, fg="green")
 
             except PermissionError:
-                click.secho(fp, fg="red")
+                click.secho(fp, fg="yellow", err=True)
diff --git a/invenio_utilities_tuw/cli/records.py b/invenio_utilities_tuw/cli/records.py
index 6fbbcb8..555aa2b 100644
--- a/invenio_utilities_tuw/cli/records.py
+++ b/invenio_utilities_tuw/cli/records.py
@@ -143,8 +143,8 @@ def update_record(metadata_file, pid, pid_type, user, patch, owners, direct):
 
         except Exception as e:
             # if that fails, try the good old plain update
-            click.secho("error: {}".format(e), fg="yellow")
-            click.secho("trying with service.update()...", fg="yellow")
+            click.secho(f"error: {e}", fg="yellow", err=True)
+            click.secho("trying with service.update()...", fg="yellow", err=True)
             service.update(id_=pid, identity=identity, data=metadata)
 
     if owners:
@@ -193,7 +193,7 @@ def list_files(pid, pid_type, user):
     for name, rec_file in record.files.entries.items():
         fi = rec_file.file
         if fi:
-            click.secho("{}\t{}\t{}".format(name, fi.uri, fi.checksum), fg="green")
+            click.secho(f"{name}\t{fi.uri}\t{fi.checksum}", fg="green")
         else:
             click.secho(name, fg="red")
 
@@ -219,12 +219,14 @@ def verify_files(pid, pid_type, user):
             click.secho(name, fg="green")
 
         else:
-            click.secho("{}: failed checksum verification".format(name), fg="red")
+            click.secho(f"{name}: failed checksum verification", fg="yellow", err=True)
             num_errors += 1
 
     if num_errors > 0:
         click.secho(
-            "{} files failed the checksum verification".format(num_errors), fg="red"
+            f"{num_errors} files failed the checksum verification",
+            fg="yellow",
+            err=True,
         )
         sys.exit(1)
 
diff --git a/invenio_utilities_tuw/cli/users.py b/invenio_utilities_tuw/cli/users.py
index 75560e4..61db784 100644
--- a/invenio_utilities_tuw/cli/users.py
+++ b/invenio_utilities_tuw/cli/users.py
@@ -33,9 +33,9 @@ def list_users(only_active, show_roles):
         users = users.filter_by(active=True)
 
     for user in users:
-        line = "{} {}".format(user.id, user.email)
+        line = f"{user.id} {user.email}"
         if show_roles:
-            line += " {}".format([r.name for r in user.roles])
+            line += f" {[r.name for r in user.roles]}"
 
         fg = "green" if user.active else "red"
         click.secho(line, fg=fg)
@@ -50,9 +50,9 @@ def show_user(user_id, show_roles):
     user = get_user_by_identifier(user_id)
     full_name = f'"{user.profile.full_name}"' if user.profile else "N/A"
 
-    line = "{} {} {}".format(user.id, user.email, full_name)
+    line = f"{user.id} {user.email} {full_name}"
     if show_roles:
-        line += " {}".format([r.name for r in user.roles])
+        line += f" {[r.name for r in user.roles]}"
 
     fg = "green" if user.active else "red"
     click.secho(line, fg=fg)
-- 
GitLab