fix: correct and standardize tabcmd 2 output messages to match tabcmd 1

tabcmd/commands/auth/session.py

@@ -262,15 +262,15 @@ def _read_existing_state(self):
             self._read_from_json()
 
     def _print_server_info(self):
-        self.logger.info("=====   Server: {}".format(self.server_url))
+        self.logger.info("  Server: {}".format(self.server_url))
         if self.proxy:
-            self.logger.info("=====   Proxy: {}".format(self.proxy))
+            self.logger.info("  Proxy: {}".format(self.proxy))
         if self.username:
-            self.logger.info("=====   Username: {}".format(self.username))
+            self.logger.info("  Username: {}".format(self.username))
         if self.certificate:
-            self.logger.info("=====   Certificate: {}".format(self.certificate))
+            self.logger.info("  Certificate: {}".format(self.certificate))
         else:
-            self.logger.info("=====   Token Name: {}".format(self.token_name))
+            self.logger.info("  Token Name: {}".format(self.token_name))
         site_display_name = self.site_name or "Default Site"
         self.logger.info(_("dataconnections.classes.tableau_server_site") + ": {}".format(site_display_name))
 
@@ -281,7 +281,7 @@ def _validate_existing_signin(self):
             if self.tableau_server and self.tableau_server.is_signed_in() and self.user_id:
                 server_user = self.tableau_server.users.get_by_id(self.user_id).name
                 if not self.username:
-                    self.logger.info("Fetched user details from server")
+                    self.logger.debug("Fetched user details from server")
                     self.username = server_user
 
                 return self.tableau_server
@@ -296,6 +296,7 @@ def _sign_in(self, tableau_auth) -> TSC.Server:
         if not self.tableau_server:
             Errors.exit_with_error(self.logger, "No server connection available for sign in")
 
+        self.logger.info(_("session.login"))
         self.logger.debug(_("session.login") + (self.server_url or ""))
         self.logger.debug(_("listsites.output").format("", self.username or self.token_name, self.site_name))
         assert self.tableau_server is not None  # Type hint for mypy

tabcmd/commands/constants.py

@@ -65,7 +65,7 @@ def exit_with_error(logger, message: Optional[str] = None, exception: Optional[E
                 Errors.log_stack(logger)
             elif exception:
                 if message:
-                    logger.info(_("tabcmd.debug.error_message") + message)
+                    logger.debug(_("tabcmd.debug.error_message") + message)
                 Errors.check_common_error_codes_and_explain(logger, exception)
             else:
                 logger.info(_("tabcmd.debug.no_exception_or_message"))

tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py

@@ -179,18 +179,22 @@ def apply_csv_options(logger, request_options: TSC.CSVRequestOptions, args):
             request_options.language = args.language
 
     @staticmethod
-    def save_to_data_file(logger, output, filename):
+    def save_to_data_file(logger, output, filename, content_name=None):
         logger.info(_("httputils.found_attachment").format(filename))
         with open(filename, "wb") as f:
             f.writelines(output)
-            logger.info(_("export.success").format("", filename))
+            # export.success renders as 'Saved <content> to "<filename>"' -- content
+            # name is the workbook/view/datasource being exported, filename is the
+            # destination path. Fall back to filename twice if no content name was
+            # threaded through (better than an empty {0}).
+            logger.info(_("export.success").format(content_name or filename, filename))
 
     @staticmethod
-    def save_to_file(logger, output, filename):
+    def save_to_file(logger, output, filename, content_name=None):
         logger.info(_("httputils.found_attachment").format(filename))
         with open(filename, "wb") as f:
             f.write(output)
-            logger.info(_("export.success").format("", filename))
+            logger.info(_("export.success").format(content_name or filename, filename))
 
     @staticmethod
     def get_custom_view_by_id(logger, server, custom_view_id) -> TSC.CustomViewItem:

tabcmd/commands/datasources_and_workbooks/delete_command.py

@@ -47,7 +47,7 @@ def run_command(cls, args):
         else:
 
             Errors.exit_with_error(logger, _("tabcmd.errors.parent.not.found"))
-        logger.info(_("delete.status").format(content_type, item_name or args.name))
+        logger.info(_("delete.status").format(content_type or "workbook", item_name or args.name))
 
         error = None
         if args.workbook or not content_type:

tabcmd/commands/datasources_and_workbooks/export_command.py

@@ -102,41 +102,45 @@ def run_command(cls, args):
             )
             Errors.exit_with_error(logger, message)
 
+        # `content_item` tracks the workbook/view/custom_view we're exporting; used
+        # for the "Saved <content-name> to '<filename>'" success message.
+        content_item = None
         try:
             if args.fullpdf:  # it's a workbook
-                workbook_item = ExportCommand.get_wb_by_content_url(logger, server, wb_content_url)
-                output = ExportCommand.download_wb_pdf(server, workbook_item, args, logger)
+                content_item = ExportCommand.get_wb_by_content_url(logger, server, wb_content_url)
+                output = ExportCommand.download_wb_pdf(server, content_item, args, logger)
 
-                default_filename = "{}.pdf".format(workbook_item.name)
+                default_filename = "{}.pdf".format(content_item.name)
 
             elif args.pdf or args.png or args.csv:  # it's a view or custom_view
                 (
-                    export_item,
+                    content_item,
                     server_content_type,
                 ) = DatasourcesWorkbooksAndViewsUrlParser.get_export_item_and_server_content_type_from_export_url(
                     view_content_url, logger, server, custom_view_id
                 )
 
                 if args.pdf:
-                    output = ExportCommand.download_view_pdf(server_content_type, export_item, args, logger)
-                    default_filename = "{}.pdf".format(export_item.name)
+                    output = ExportCommand.download_view_pdf(server_content_type, content_item, args, logger)
+                    default_filename = "{}.pdf".format(content_item.name)
                 elif args.csv:
-                    output = ExportCommand.download_csv(server_content_type, export_item, args, logger)
-                    default_filename = "{}.csv".format(export_item.name)
+                    output = ExportCommand.download_csv(server_content_type, content_item, args, logger)
+                    default_filename = "{}.csv".format(content_item.name)
                 elif args.png:
-                    output = ExportCommand.download_png(server_content_type, export_item, args, logger)
-                    default_filename = "{}.png".format(export_item.name)
+                    output = ExportCommand.download_png(server_content_type, content_item, args, logger)
+                    default_filename = "{}.png".format(content_item.name)
 
         except TSC.ServerResponseError as e:
             Errors.exit_with_error(logger, _("publish.errors.unexpected_server_response").format(""), exception=e)
         except Exception as e:
             Errors.exit_with_error(logger, exception=e)
         try:
             save_name = args.filename or default_filename
+            content_name = content_item.name if content_item is not None else None
             if args.csv:
-                ExportCommand.save_to_data_file(logger, output, save_name)
+                ExportCommand.save_to_data_file(logger, output, save_name, content_name=content_name)
             else:
-                ExportCommand.save_to_file(logger, output, save_name)
+                ExportCommand.save_to_file(logger, output, save_name, content_name=content_name)
 
         except Exception as e:
             Errors.exit_with_error(logger, "Error saving to file", exception=e)

tabcmd/commands/datasources_and_workbooks/get_url_command.py

@@ -106,7 +106,7 @@ def generate_pdf(logger, server_content_type, args, get_url_item):
             DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_pdf, args.url)
             server_content_type.populate_pdf(get_url_item, req_option_pdf)
             filename = GetUrl.filename_from_args(args.filename, get_url_item.name, "pdf")
-            DatasourcesAndWorkbooks.save_to_file(logger, get_url_item.pdf, filename)
+            DatasourcesAndWorkbooks.save_to_file(logger, get_url_item.pdf, filename, content_name=get_url_item.name)
         except Exception as e:
             Errors.exit_with_error(logger, exception=e)
 
@@ -119,7 +119,7 @@ def generate_png(logger, server_content_type, args, get_url_item):
             DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_png, args.url)
             server_content_type.populate_image(get_url_item, req_option_png)
             filename = GetUrl.filename_from_args(args.filename, get_url_item.name, "png")
-            DatasourcesAndWorkbooks.save_to_file(logger, get_url_item.image, filename)
+            DatasourcesAndWorkbooks.save_to_file(logger, get_url_item.image, filename, content_name=get_url_item.name)
         except Exception as e:
             Errors.exit_with_error(logger, exception=e)
 
@@ -132,7 +132,9 @@ def generate_csv(logger, server_content_type, args, get_url_item):
             DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_csv, args.url)
             server_content_type.populate_csv(get_url_item, req_option_csv)
             file_name_with_path = GetUrl.filename_from_args(args.filename, get_url_item.name, "csv")
-            DatasourcesAndWorkbooks.save_to_data_file(logger, get_url_item.csv, file_name_with_path)
+            DatasourcesAndWorkbooks.save_to_data_file(
+                logger, get_url_item.csv, file_name_with_path, content_name=get_url_item.name
+            )
         except Exception as e:
             Errors.exit_with_error(logger, exception=e)
 

tabcmd/commands/group/delete_group_command.py

@@ -29,7 +29,7 @@ def run_command(cls, args):
         try:
             logger.info(_("tabcmd.find.group").format(args.name))
             group_id = Server.find_group(logger, server, args.name).id
-            logger.info(_("deletegroup.status").format(group_id))
+            logger.info(_("deletegroup.status").format(args.name))
             server.groups.delete(group_id)
             logger.info(_("common.output.succeeded"))
         except Exception as e:

tabcmd/commands/site/list_sites_command.py

@@ -31,7 +31,7 @@ def run_command(cls, args):
             sites, pagination = server.sites.get()
             logger.info(_("listsites.status").format(session.username))
             for site in sites:
-                logger.info(_("listsites.output").format(" ", site.name, site.id))
+                logger.info(_("listsites.output").format(" ", site.name, site.content_url))
                 if args.get_extract_encryption_mode:
                     logger.info("EXTRACTENCRYPTION: {}".format(site.extract_encryption_mode))
         except Exception as e:

tabcmd/commands/user/create_site_users.py

@@ -62,7 +62,7 @@ def run_command(cls, args):
                     logger.debug(type(e))
                     number_of_errors += 1
                     logger.debug(number_of_errors)
-                    error_list.append(e.__class__.__name__)  # + ": " + e.__cause__ or "Unknown")
+                    error_list.append(str(e))
                 logger.debug(error_list)
         logger.info(_("session.monitorjob.percent_complete").format(100))
         logger.info(_("importcsvsummary.line.processed").format(number_of_users_listed))

tabcmd/commands/user/remove_users_command.py

@@ -27,6 +27,6 @@ def run_command(cls, args):
         session = Session()
         server = session.create_session(args, logger)
 
-        logger.info(_("tabcmd.removeusers.server").format(args.users.name, args.name))
+        logger.info(_("tabcmd.removeusers.group").format(args.users.name, args.name))
 
         UserCommand.act_on_users(logger, server, "removed", server.groups.remove_user, args)

tabcmd/execution/logger_config.py

@@ -7,10 +7,16 @@
 
 path = os.path.dirname(os.path.abspath(__file__))
 
+# tabcmd Classic prefixes every INFO line with "=====". This PR's goal is parity
+# with Classic output, so the prefix is on by default. Users who prefer the
+# plainer tabcmd 2 style can opt out with TABCMD_CLASSIC_OUTPUT=false (or 0/no).
+_CLASSIC_OUTPUT = os.environ.get("TABCMD_CLASSIC_OUTPUT", "true").lower() not in ("0", "false", "no")
+_INFO_FORMAT = "===== %(message)-30s" if _CLASSIC_OUTPUT else "%(message)-30s"
+
 FORMATS = {
     logging.ERROR: "%(asctime)s %(levelname)-5s:(%(name)-10s %(filename)-10s: %(lineno)d): %(message)-30s",
     logging.WARN: "%(asctime)s %(levelname)-5s: (%(name)-10s %(filename)-10s: %(lineno)d): %(message)-30s",
-    logging.INFO: "%(message)-30s",
+    logging.INFO: _INFO_FORMAT,
     logging.DEBUG: "%(asctime)s %(levelname)-5s: (%(name)-10s %(filename)-10s: %(lineno)d): %(message)-30s",
 }
 

tabcmd/execution/parent_parser.py

@@ -198,5 +198,5 @@ def __init__(self, _parser: ParentParser):
 
     def run_command(self, args):
         logger = log(__name__, "info")
-        logger.info(f"{_('tabcmd.name')} {version}\n")
-        logger.info(self.parser.root.format_help())
+        print(f"{_('tabcmd.name')} {version}\n")
+        print(self.parser.root.format_help())

tabcmd/execution/tabcmd_controller.py

@@ -32,11 +32,8 @@ def run(parser, user_input=None):
             parser.print_help()
             sys.exit(0)
 
-        if hasattr(namespace, "logging_level") and namespace.logging_level != logging.INFO:
-            print("logging:", namespace.logging_level)
-
-        logger = log(__name__, namespace.logging_level or logging.INFO)
-        logger.info("Tabcmd {}".format(version))
+        logger = log(__name__, namespace.logging_level or "INFO")
+        print("Tabcmd {}".format(version))
         if hasattr(namespace, "password") or hasattr(namespace, "token_value"):
             # don't print whole namespace because it has secrets
             logger.debug(namespace.func)

tabcmd/locales/en/tabcmd_messages_en.properties

@@ -166,7 +166,7 @@ tabcmd.global.help.page_size=Specify the page size for query results
 tabcmd.global.help.skip_connection_check=Skip connection check: do not validate the connection during publishing
 tabcmd.howto=Run a specific command
 tabcmd.launching=Launching tabcmd
-tabcmd.listing.header====== Listing {0} content for user {1}...
+tabcmd.listing.header= Listing {0} content for user {1}...
 tabcmd.listing.label.id=ID: {}
 tabcmd.listing.label.name=\tNAME: {}
 tabcmd.listing.none=No content found
@@ -204,6 +204,7 @@ tabcmd.publish.options.tabbed.detailed=Publish with tabbed views enabled. Each s
 tabcmd.refresh.options.bridge=Refresh datasource through Tableau Bridge
 tabcmd.removeusers.help.group_name=The group to remove users from
 tabcmd.removeusers.server=Removing users listed in {0} from the server...
+tabcmd.removeusers.group=Removing users listed in {0} from the group ''{1}''...
 tabcmd.report.error.user_csv.at_char=If a user name includes an @ character that represents anything other than a domain separator, you need to refer to the symbol using the hexadecimal format: \\0x40
 tabcmd.report.error.user_csv.too_many_columns=The file contains {0} columns, but there are only {1} valid columns in a user import csv file
 tabcmd.report.error.user.no_spaces_in_username=Username cannot contain spaces

tests/commands/test_geturl_utils.py

@@ -448,6 +448,31 @@ def test_save_to_data_file(self):
         filename = "test_out.csv"
         ExportCommand.save_to_data_file(mock_logger, mock_content, filename)
 
+    def test_save_to_file_uses_content_name_when_supplied(self):
+        # tabcmd 1 prints "Saved <content-name> to '<filename>'" -- the content
+        # name is a distinct value from the destination filename. Passing them
+        # both as the filename was a bug.
+        logger = mock.MagicMock()
+        with mock.patch(
+            "tabcmd.commands.datasources_and_workbooks.datasources_and_workbooks_command._",
+            side_effect=lambda k: "Saved {0} to '{1}'" if k == "export.success" else k,
+        ):
+            ExportCommand.save_to_file(logger, bytes(), "Regional.pdf", content_name="Regional Sales")
+        rendered = [c[0][0] for c in logger.info.call_args_list]
+        assert "Saved Regional Sales to 'Regional.pdf'" in rendered, rendered
+
+    def test_save_to_file_falls_back_to_filename_when_no_content_name(self):
+        # If a caller doesn't supply content_name we still want a usable message;
+        # falling back to filename is better than an empty {0} placeholder.
+        logger = mock.MagicMock()
+        with mock.patch(
+            "tabcmd.commands.datasources_and_workbooks.datasources_and_workbooks_command._",
+            side_effect=lambda k: "Saved {0} to '{1}'" if k == "export.success" else k,
+        ):
+            ExportCommand.save_to_file(logger, bytes(), "test_out.pdf")
+        rendered = [c[0][0] for c in logger.info.call_args_list]
+        assert "Saved test_out.pdf to 'test_out.pdf'" in rendered, rendered
+
 
 class FilenameExtensionTests(unittest.TestCase):
     # get_file_type_from_filename(logger, url, file_name)