From 510c2c3eee06f48b421992316d3118ce6a3f9565 Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Wed, 1 Mar 2023 17:24:33 -0800
Subject: [PATCH 1/7] implement thumbnail options.

including 'not yet implemented' message for --thumbnail-group
---
 .../datasources_and_workbooks/publish_command.py  | 13 ++++++++++++-
 tabcmd/execution/global_options.py                | 15 +++++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py
index ce04fa31..c0b85262 100644
--- a/tabcmd/commands/datasources_and_workbooks/publish_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py
@@ -66,18 +66,29 @@ def run_command(args):
         source = PublishCommand.get_filename_extension_if_tableau_type(logger, args.filename)
         logger.info(_("publish.status").format(args.filename))
         if source in ["twbx", "twb"]:
-            new_workbook = TSC.WorkbookItem(project_id, name=args.name, show_tabs=args.tabbed)
+            if args.thumbnail_group:
+                raise AttributeError("Generating thumbnails for a group is not yet implemented.")
+            if args.thumbnail_username and args.thumbnail_group:
+                raise AttributeError("Cannot specify both a user and group for thumbnails.")
+
+            new_workbook = TSC.WorkbookItem(
+                project_id, name=args.name, show_tabs=args.tabbed)
             try:
                 new_workbook = server.workbooks.publish(
                     new_workbook,
                     args.filename,
                     publish_mode,
+                    args.thumbnail_user,
+                    args.thumbnail_group,
                     connection_credentials=creds,
                     as_job=False,
                     skip_connection_check=False,
                 )
+            except IOError as ioe:
+                Errors.exit_with_error(logger, ioe)
             except Exception as e:
                 Errors.exit_with_error(logger, e)
+
             logger.info(_("publish.success") + "\n{}".format(new_workbook.webpage_url))
 
         elif source in ["tds", "tdsx", "hyper"]:
diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py
index 60c72fd0..05c44700 100644
--- a/tabcmd/execution/global_options.py
+++ b/tabcmd/execution/global_options.py
@@ -311,9 +311,20 @@ def set_publish_args(parser):
         action="store_true",
         help="Encrypt extracts in the workbook, datasource, or extract being published to the server",
     )
+
+    parser.add_argument("--oauth-username", help="The email address of a preconfigured OAuth connection")
+    parser.add_argument("--save-oauth", action="store_true", help="Save embedded OAuth credentials in the datasource")
+
+    # These two only apply for a workbook, not a datasource
     thumbnails = parser.add_mutually_exclusive_group()
-    thumbnails.add_argument("--thumbnail-username", help="Not yet implemented")
-    thumbnails.add_argument("--thumbnail-group", help="Not yet implemented")  # not implemented in the REST API
+    thumbnails.add_argument(
+        "--thumbnail-username",
+        help="If the workbook contains user filters, the thumbnails will be generated based on what the "
+             "specified user can see. Cannot be specified when --thumbnail-group option is set.")
+    thumbnails.add_argument(
+        "--thumbnail-group",
+        help="If the workbook contains user filters, the thumbnails will be generated based on what the "
+             "specified group can see. Cannot be specified when --thumbnail-usernameoption is set.")
 
     parser.add_argument("--use-tableau-bridge", action="store_true", help="Refresh datasource through Tableau Bridge")
 

From 151a90a399a9d58246a6396b85f069e1912b7ad2 Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Wed, 1 Mar 2023 18:33:03 -0800
Subject: [PATCH 2/7] Implement overwrite/append/replace

- tweaked logic of which value can apply
- clarified help content
- added tests for all valid publish command options

Not tested.
---
 .../publish_command.py                        | 18 ++++-
 tabcmd/execution/global_options.py            | 52 ++++++++------
 tabcmd/execution/parent_parser.py             |  2 +-
 tests/commands/test_run_commands.py           |  4 ++
 tests/parsers/test_parser_publish.py          | 67 +++++++++++++++++++
 5 files changed, 118 insertions(+), 25 deletions(-)

diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py
index c0b85262..4450f92a 100644
--- a/tabcmd/commands/datasources_and_workbooks/publish_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py
@@ -29,6 +29,8 @@ def define_args(publish_parser):
         )
         set_publish_args(group)
         set_project_r_arg(group)
+        set_overwrite_option(group)
+        set_append_replace_option(group)
         set_parent_project_arg(group)
 
     @staticmethod
@@ -52,7 +54,9 @@ def run_command(args):
             args.project_name = "default"
             args.parent_project_path = ""
 
-        publish_mode = PublishCommand.get_publish_mode(args)  # --overwrite, --replace
+        publish_mode = PublishCommand.get_publish_mode(args)  # --overwrite, --replace, --append
+        if not publish_mode:
+            Errors.exit_with_error(logger, "Invalid combination of publishing options (Append, Overwrite, Replace)")
         logger.info("Publishing as " + publish_mode)
 
         if args.db_username:
@@ -78,11 +82,12 @@ def run_command(args):
                     new_workbook,
                     args.filename,
                     publish_mode,
-                    args.thumbnail_user,
+                    args.thumbnail_username,
                     args.thumbnail_group,
                     connection_credentials=creds,
                     as_job=False,
                     skip_connection_check=False,
+
                 )
             except IOError as ioe:
                 Errors.exit_with_error(logger, ioe)
@@ -104,8 +109,15 @@ def run_command(args):
 
     @staticmethod
     def get_publish_mode(args):
-        if args.overwrite:
+        if args.append:
+            # only relevant for datasources: Append the extract file to the existing data source.
+            publish_mode = TSC.Server.PublishMode.Append
+        if args.replace or args.overwrite:
+            if args.append:
+                return None  # invalid combination
+            # Overwrites the workbook, data source, or data extract if it already exists on the server.
             publish_mode = TSC.Server.PublishMode.Overwrite
         else:
+            # default: fail if it already exists on the server
             publish_mode = TSC.Server.PublishMode.CreateNew
         return publish_mode
diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py
index 05c44700..3bc55970 100644
--- a/tabcmd/execution/global_options.py
+++ b/tabcmd/execution/global_options.py
@@ -126,7 +126,7 @@ def set_encryption_option(parser):
         "--encrypt",
         dest="encrypt",
         action="store_true",  # set to true IF user passes in option --encrypt
-        help="Encrypt the newly created extract.",
+        help="Encrypt the newly created extract. [N/a on Tableau Cloud: extracts are always encrypted]",
     )
     return parser
 
@@ -230,7 +230,8 @@ def set_common_site_args(parser):
         "--extract-encryption-mode",
         choices=encryption_modes,
         type=case_insensitive_string_type(encryption_modes),
-        help="The extract encryption mode for the site can be enforced, enabled or disabled. ",
+        help="The extract encryption mode for the site can be enforced, enabled or disabled. "
+             "[N/a on Tableau Cloud: encryption mode is always enforced] ",
     )
 
     parser.add_argument(
@@ -275,8 +276,6 @@ def set_filename_arg(parser, description=_("get.options.file")):
 def set_publish_args(parser):
     parser.add_argument("-n", "--name", help="Name to publish the new datasource or workbook by.")
 
-    set_overwrite_option(parser)
-
     creds = parser.add_mutually_exclusive_group()
     creds.add_argument("--oauth-username", help="The email address of a preconfigured OAuth connection")
     creds.add_argument(
@@ -301,20 +300,15 @@ def set_publish_args(parser):
         help="When a workbook with tabbed views is published, each sheet becomes a tab that viewers can use to \
         navigate through the workbook",
     )
-    parser.add_argument(
-        "--replace", action="store_true", help="Use the extract file to replace the existing data source."
-    )
-    parser.add_argument("--disable-uploader", action="store_true", help="Disable the incremental file uploader.")
-    parser.add_argument("--restart", help="Restart the file upload.")
+    parser.add_argument("--disable-uploader", action="store_true", help="[DEPRECATED - has no effect] Disable the incremental file uploader.")
+    parser.add_argument("--restart", help="[DEPRECATED - has no effect] Restart the file upload.")
     parser.add_argument(
         "--encrypt-extracts",
         action="store_true",
-        help="Encrypt extracts in the workbook, datasource, or extract being published to the server",
+        help="Encrypt extracts in the workbook, datasource, or extract being published to the server. "
+             "[N/a on Tableau Cloud: extracts are always encrypted]",
     )
 
-    parser.add_argument("--oauth-username", help="The email address of a preconfigured OAuth connection")
-    parser.add_argument("--save-oauth", action="store_true", help="Save embedded OAuth credentials in the datasource")
-
     # These two only apply for a workbook, not a datasource
     thumbnails = parser.add_mutually_exclusive_group()
     thumbnails.add_argument(
@@ -323,24 +317,40 @@ def set_publish_args(parser):
              "specified user can see. Cannot be specified when --thumbnail-group option is set.")
     thumbnails.add_argument(
         "--thumbnail-group",
-        help="If the workbook contains user filters, the thumbnails will be generated based on what the "
-             "specified group can see. Cannot be specified when --thumbnail-usernameoption is set.")
+        help="[Not yet implemented] If the workbook contains user filters, the thumbnails will be generated based on what the "
+             "specified group can see. Cannot be specified when --thumbnail-username option is set.")
 
     parser.add_argument("--use-tableau-bridge", action="store_true", help="Refresh datasource through Tableau Bridge")
 
 
-def set_overwrite_option(parser):
+# these two are used to publish an extract to an existing data source
+def set_append_replace_option(parser):
     append_group = parser.add_mutually_exclusive_group()
     append_group.add_argument(
-        "-o",
-        "--overwrite",
+        "--append",
         action="store_true",
-        help="Overwrites the workbook, data source, or data extract if it already exists on the server.",
+        help="Set to true to append the data being published to an existing data source that has the same name. "
+             "The default behavior is to fail if the data source already exists. "
+             "If append is set to true but the data source doesn't already exist, the operation fails."
     )
+
+    # what's the difference between this and 'overwrite'?
+    # This is meant for when a) the local file is an extract b) the server item is an existing data source
     append_group.add_argument(
-        "--append",
+        "--replace",
+        action="store_true",
+        help="Use the extract file being published to replace data in the existing data source. The default "
+             "behavior is to fail if the item already exists."
+    )
+
+# this is meant to be like replacing like
+def set_overwrite_option(parser):
+    parser.add_argument(
+        "-o",
+        "--overwrite",
         action="store_true",
-        help="Append the extract file to the existing data source.",
+        help="Overwrites the workbook, data source, or data extract if it already exists on the server. The default "
+             "behavior is to fail if the item already exists."
     )
 
 
diff --git a/tabcmd/execution/parent_parser.py b/tabcmd/execution/parent_parser.py
index 21890728..ba379619 100644
--- a/tabcmd/execution/parent_parser.py
+++ b/tabcmd/execution/parent_parser.py
@@ -101,7 +101,7 @@ def parent_parser_with_global_options():
     parser.add_argument(
         "--continue-if-exists",
         action="store_true",  # default behavior matches old tabcmd
-        help=strings[9],
+        help=strings[9],  # kind of equivalent to 'overwrite' in the publish command
     )
 
     parser.add_argument(
diff --git a/tests/commands/test_run_commands.py b/tests/commands/test_run_commands.py
index 5e6ed25a..a52835a3 100644
--- a/tests/commands/test_run_commands.py
+++ b/tests/commands/test_run_commands.py
@@ -148,6 +148,10 @@ def test_publish(self, mock_session, mock_server):
         mock_args.tabbed = True
         mock_args.db_username = None
         mock_args.oauth_username = None
+        mock_args.append = False
+        mock_args.replace = False
+        mock_args.thumbnail_username = None
+        mock_args.thumbnail_group = None
         mock_server.projects = getter
         publish_command.PublishCommand.run_command(mock_args)
         mock_session.assert_called()
diff --git a/tests/parsers/test_parser_publish.py b/tests/parsers/test_parser_publish.py
index 261aeb6e..417affb7 100644
--- a/tests/parsers/test_parser_publish.py
+++ b/tests/parsers/test_parser_publish.py
@@ -1,4 +1,5 @@
 import unittest
+from unittest import skip
 
 from tabcmd.commands.datasources_and_workbooks.publish_command import PublishCommand
 from .common_setup import *
@@ -38,3 +39,69 @@ def test_publish_parser_save_password(self):
         ]
         args = self.parser_under_test.parse_args(mock_args)
         assert args.save_db_password is True, args
+
+    def test_publish_parser_save_oauth(self):
+        mock_args = [
+            commandname,
+            "filename.twbx",
+            "--oauth-username",
+            "user",
+            "--save-oauth",
+        ]
+        args = self.parser_under_test.parse_args(mock_args)
+        assert args.save_oauth is True, args
+        assert args.oauth_username == "user", args
+
+
+    def test_publish_parser_thumbnails(self):
+        mock_args = [commandname, "filename.twbx", "--thumbnail-username"] # no value for thumbnail-user
+        with self.assertRaises(SystemExit):
+            args = self.parser_under_test.parse_args(mock_args)
+
+        mock_args = [commandname, "filename.twbx", "--thumbnail-username", "goofy"]
+        args = self.parser_under_test.parse_args(mock_args)
+        assert args.thumbnail_username == "goofy", args
+
+    @skip("Not yet implemented")
+    def test_publish_parser_thumbnail_group(self):
+        mock_args = [commandname, "filename.twbx", "--thumbnail-group", "goofy"]
+        args = self.parser_under_test.parse_args(mock_args)
+        assert args.thumbnail_group == "goofy", args
+
+    """
+    append | replace | overwrite -> result
+    --------
+    true   | F/empty | F/empty   -> append
+    true   | F/empty | true      -> ERROR
+    true   | true    | F/empty   -> ERROR
+    .... basically, replace == overwrite, append != r/o
+    """
+    def test_publish_parser_append_options(self):
+        mock_args = [commandname, "filename.twbx", "--append"]
+        args = self.parser_under_test.parse_args(mock_args)
+
+    def test_publish_parser_replace_and_append(self):
+        mock_args = [commandname, "filename.twbx", "--append", "--replace"]
+        with self.assertRaises(SystemExit):
+            args = self.parser_under_test.parse_args(mock_args)
+
+    def test_publish_parser_replace_options(self):
+        mock_args = [commandname, "filename.twbx", "--overwrite"]
+        args = self.parser_under_test.parse_args(mock_args)
+
+        mock_args = [commandname, "filename.twbx", "--replace"]
+        args = self.parser_under_test.parse_args(mock_args)
+
+        mock_args = [commandname, "filename.twbx", "--replace", "--overwrite"]
+        args = self.parser_under_test.parse_args(mock_args)
+
+    def test_publish_parser_deprecated_options(self):
+        # does nothing in new tabcmd, but shouldn't break anything
+        mock_args = [commandname, "filename.twbx", "--disable-uploader"]
+        args = self.parser_under_test.parse_args(mock_args)
+        mock_args = [commandname, "filename.twbx", "--restart", "argument"]
+        args = self.parser_under_test.parse_args(mock_args)
+
+    def test_publish_parser_use_bridge_option(self):
+        mock_args = [commandname, "filename.twbx","--use-tableau-bridge"]
+        args = self.parser_under_test.parse_args(mock_args)

From 004b546017c38c6e169ca8fc33d2676868ec42ae Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Thu, 2 Mar 2023 14:16:22 -0800
Subject: [PATCH 3/7] edit encryption help

---
 tabcmd/execution/global_options.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py
index 3bc55970..6a7f50f5 100644
--- a/tabcmd/execution/global_options.py
+++ b/tabcmd/execution/global_options.py
@@ -126,7 +126,7 @@ def set_encryption_option(parser):
         "--encrypt",
         dest="encrypt",
         action="store_true",  # set to true IF user passes in option --encrypt
-        help="Encrypt the newly created extract. [N/a on Tableau Cloud: extracts are always encrypted]",
+        help="Encrypt the newly created extract. [N/a on Tableau Cloud: extract encryption is controlled by Site Admin]",
     )
     return parser
 
@@ -306,7 +306,7 @@ def set_publish_args(parser):
         "--encrypt-extracts",
         action="store_true",
         help="Encrypt extracts in the workbook, datasource, or extract being published to the server. "
-             "[N/a on Tableau Cloud: extracts are always encrypted]",
+             "[N/a on Tableau Cloud: extract encryption is controlled by Site Admin]",
     )
 
     # These two only apply for a workbook, not a datasource
@@ -372,12 +372,12 @@ def set_calculations_options(parser):
     calc_group.add_argument(
         "--addcalculations",
         action="store_true",
-        help="DEPRECATED [has no effect] Add precalculated data operations in the extract data source.",
+        help="[Not implemented] Add precalculated data operations in the extract data source.",
     )
     calc_group.add_argument(
         "--removecalculations",
         action="store_true",
-        help="DEPRECATED [has no effect] Remove precalculated data in the extract data source.",
+        help="[Not implemented] Remove precalculated data in the extract data source.",
     )
     return calc_group
 

From ee101567a7d98b2339b1c891d9df07e56c805d02 Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Sat, 4 Mar 2023 13:12:54 -0800
Subject: [PATCH 4/7] format

---
 .../publish_command.py                        |  4 +--
 tabcmd/execution/global_options.py            | 25 ++++++++++++-------
 tests/parsers/test_parser_publish.py          |  6 ++---
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py
index 4450f92a..981bbb53 100644
--- a/tabcmd/commands/datasources_and_workbooks/publish_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py
@@ -75,8 +75,7 @@ def run_command(args):
             if args.thumbnail_username and args.thumbnail_group:
                 raise AttributeError("Cannot specify both a user and group for thumbnails.")
 
-            new_workbook = TSC.WorkbookItem(
-                project_id, name=args.name, show_tabs=args.tabbed)
+            new_workbook = TSC.WorkbookItem(project_id, name=args.name, show_tabs=args.tabbed)
             try:
                 new_workbook = server.workbooks.publish(
                     new_workbook,
@@ -87,7 +86,6 @@ def run_command(args):
                     connection_credentials=creds,
                     as_job=False,
                     skip_connection_check=False,
-
                 )
             except IOError as ioe:
                 Errors.exit_with_error(logger, ioe)
diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py
index 6a7f50f5..695820db 100644
--- a/tabcmd/execution/global_options.py
+++ b/tabcmd/execution/global_options.py
@@ -231,7 +231,7 @@ def set_common_site_args(parser):
         choices=encryption_modes,
         type=case_insensitive_string_type(encryption_modes),
         help="The extract encryption mode for the site can be enforced, enabled or disabled. "
-             "[N/a on Tableau Cloud: encryption mode is always enforced] ",
+        "[N/a on Tableau Cloud: encryption mode is always enforced] ",
     )
 
     parser.add_argument(
@@ -300,13 +300,17 @@ def set_publish_args(parser):
         help="When a workbook with tabbed views is published, each sheet becomes a tab that viewers can use to \
         navigate through the workbook",
     )
-    parser.add_argument("--disable-uploader", action="store_true", help="[DEPRECATED - has no effect] Disable the incremental file uploader.")
+    parser.add_argument(
+        "--disable-uploader",
+        action="store_true",
+        help="[DEPRECATED - has no effect] Disable the incremental file uploader.",
+    )
     parser.add_argument("--restart", help="[DEPRECATED - has no effect] Restart the file upload.")
     parser.add_argument(
         "--encrypt-extracts",
         action="store_true",
         help="Encrypt extracts in the workbook, datasource, or extract being published to the server. "
-             "[N/a on Tableau Cloud: extract encryption is controlled by Site Admin]",
+        "[N/a on Tableau Cloud: extract encryption is controlled by Site Admin]",
     )
 
     # These two only apply for a workbook, not a datasource
@@ -314,11 +318,13 @@ def set_publish_args(parser):
     thumbnails.add_argument(
         "--thumbnail-username",
         help="If the workbook contains user filters, the thumbnails will be generated based on what the "
-             "specified user can see. Cannot be specified when --thumbnail-group option is set.")
+        "specified user can see. Cannot be specified when --thumbnail-group option is set.",
+    )
     thumbnails.add_argument(
         "--thumbnail-group",
         help="[Not yet implemented] If the workbook contains user filters, the thumbnails will be generated based on what the "
-             "specified group can see. Cannot be specified when --thumbnail-username option is set.")
+        "specified group can see. Cannot be specified when --thumbnail-username option is set.",
+    )
 
     parser.add_argument("--use-tableau-bridge", action="store_true", help="Refresh datasource through Tableau Bridge")
 
@@ -330,8 +336,8 @@ def set_append_replace_option(parser):
         "--append",
         action="store_true",
         help="Set to true to append the data being published to an existing data source that has the same name. "
-             "The default behavior is to fail if the data source already exists. "
-             "If append is set to true but the data source doesn't already exist, the operation fails."
+        "The default behavior is to fail if the data source already exists. "
+        "If append is set to true but the data source doesn't already exist, the operation fails.",
     )
 
     # what's the difference between this and 'overwrite'?
@@ -340,9 +346,10 @@ def set_append_replace_option(parser):
         "--replace",
         action="store_true",
         help="Use the extract file being published to replace data in the existing data source. The default "
-             "behavior is to fail if the item already exists."
+        "behavior is to fail if the item already exists.",
     )
 
+
 # this is meant to be like replacing like
 def set_overwrite_option(parser):
     parser.add_argument(
@@ -350,7 +357,7 @@ def set_overwrite_option(parser):
         "--overwrite",
         action="store_true",
         help="Overwrites the workbook, data source, or data extract if it already exists on the server. The default "
-             "behavior is to fail if the item already exists."
+        "behavior is to fail if the item already exists.",
     )
 
 
diff --git a/tests/parsers/test_parser_publish.py b/tests/parsers/test_parser_publish.py
index 417affb7..65aaa20d 100644
--- a/tests/parsers/test_parser_publish.py
+++ b/tests/parsers/test_parser_publish.py
@@ -52,9 +52,8 @@ def test_publish_parser_save_oauth(self):
         assert args.save_oauth is True, args
         assert args.oauth_username == "user", args
 
-
     def test_publish_parser_thumbnails(self):
-        mock_args = [commandname, "filename.twbx", "--thumbnail-username"] # no value for thumbnail-user
+        mock_args = [commandname, "filename.twbx", "--thumbnail-username"]  # no value for thumbnail-user
         with self.assertRaises(SystemExit):
             args = self.parser_under_test.parse_args(mock_args)
 
@@ -76,6 +75,7 @@ def test_publish_parser_thumbnail_group(self):
     true   | true    | F/empty   -> ERROR
     .... basically, replace == overwrite, append != r/o
     """
+
     def test_publish_parser_append_options(self):
         mock_args = [commandname, "filename.twbx", "--append"]
         args = self.parser_under_test.parse_args(mock_args)
@@ -103,5 +103,5 @@ def test_publish_parser_deprecated_options(self):
         args = self.parser_under_test.parse_args(mock_args)
 
     def test_publish_parser_use_bridge_option(self):
-        mock_args = [commandname, "filename.twbx","--use-tableau-bridge"]
+        mock_args = [commandname, "filename.twbx", "--use-tableau-bridge"]
         args = self.parser_under_test.parse_args(mock_args)

From 3965e1d4a24dfa03356af009848d2446dc331861 Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Thu, 9 Mar 2023 00:09:18 -0800
Subject: [PATCH 5/7] lots of error log cleanup

better stacktrace, less repetition
---
 tabcmd/commands/constants.py                  | 22 +++++---
 .../publish_command.py                        | 53 +++++++++++--------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/tabcmd/commands/constants.py b/tabcmd/commands/constants.py
index c491a135..bf633560 100644
--- a/tabcmd/commands/constants.py
+++ b/tabcmd/commands/constants.py
@@ -1,6 +1,8 @@
 import inspect
 import sys
 
+import tableauserverclient
+
 from tabcmd.execution.localize import _
 
 
@@ -44,7 +46,7 @@ def log_stack(logger):
             stack = inspect.stack()
             here = stack[0]
             file, line, func = here[1:4]
-            start = 0
+            start = 1
             n_lines = 5
             logger.debug(HEADER_FMT % (file, func))
             for frame in stack[start + 1 : n_lines]:
@@ -54,15 +56,17 @@ def log_stack(logger):
             logger.info("Error printing stack trace:", e)
 
     @staticmethod
-    def exit_with_error(logger, message=None, exception=None):
+    def exit_with_error(logger, message=None, exception:Exception=None):
         try:
-            Errors.log_stack(logger)
             if message and not exception:
                 logger.error(message)
-            if exception:
+                Errors.log_stack(logger)
+            elif exception:
                 if message:
-                    logger.debug("Error message: " + message)
-            Errors.check_common_error_codes_and_explain(logger, exception)
+                    logger.info("Error message: " + message)
+                Errors.check_common_error_codes_and_explain(logger, exception)
+            else:
+                logger.info("No exception or message provided")
         except Exception as exc:
             print(sys.stderr, "Error during log call from exception - {} {}".format(exc.__class__, message))
         try:
@@ -72,7 +76,7 @@ def exit_with_error(logger, message=None, exception=None):
         sys.exit(1)
 
     @staticmethod
-    def check_common_error_codes_and_explain(logger, exception):
+    def check_common_error_codes_and_explain(logger, exception: Exception):
         # most errors contain as much info in the message as we can get from the code
         # identify any that we can add useful detail for and include them here
         if Errors.is_expired_session(exception):
@@ -83,5 +87,7 @@ def check_common_error_codes_and_explain(logger, exception):
             # "session.session_expired_login"))
             # session.renew_session()
             return
-        else:
+        if exception.__class__ == tableauserverclient.ServerResponseError:
             logger.error(exception)
+        else:
+            logger.exception(exception)
diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py
index 981bbb53..2df9927e 100644
--- a/tabcmd/commands/datasources_and_workbooks/publish_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py
@@ -54,10 +54,7 @@ def run_command(args):
             args.project_name = "default"
             args.parent_project_path = ""
 
-        publish_mode = PublishCommand.get_publish_mode(args)  # --overwrite, --replace, --append
-        if not publish_mode:
-            Errors.exit_with_error(logger, "Invalid combination of publishing options (Append, Overwrite, Replace)")
-        logger.info("Publishing as " + publish_mode)
+        publish_mode = PublishCommand.get_publish_mode(args, logger)
 
         if args.db_username:
             creds = TSC.models.ConnectionCredentials(args.db_username, args.db_password, embed=args.save_db_password)
@@ -77,20 +74,19 @@ def run_command(args):
 
             new_workbook = TSC.WorkbookItem(project_id, name=args.name, show_tabs=args.tabbed)
             try:
+                print(creds)
                 new_workbook = server.workbooks.publish(
                     new_workbook,
                     args.filename,
                     publish_mode,
-                    args.thumbnail_username,
-                    args.thumbnail_group,
+                    # args.thumbnail_username, not yet implemented in tsc
+                    # args.thumbnail_group,
                     connection_credentials=creds,
                     as_job=False,
                     skip_connection_check=False,
                 )
-            except IOError as ioe:
-                Errors.exit_with_error(logger, ioe)
             except Exception as e:
-                Errors.exit_with_error(logger, e)
+                Errors.exit_with_error(logger, exception=e)
 
             logger.info(_("publish.success") + "\n{}".format(new_workbook.webpage_url))
 
@@ -102,20 +98,35 @@ def run_command(args):
                     new_datasource, args.filename, publish_mode, connection_credentials=creds
                 )
             except Exception as exc:
-                Errors.exit_with_error(logger, exc)
+                Errors.exit_with_error(logger, exception=exc)
             logger.info(_("publish.success") + "\n{}".format(new_datasource.webpage_url))
 
+
+    # todo write tests for this method
     @staticmethod
-    def get_publish_mode(args):
+    def get_publish_mode(args, logger):
+        # default: fail if it already exists on the server
+        default_mode = TSC.Server.PublishMode.CreateNew
+        publish_mode = default_mode
+
+        if args.replace:
+            raise AttributeError("Replacing an extract is not yet implemented")
+
         if args.append:
-            # only relevant for datasources: Append the extract file to the existing data source.
-            publish_mode = TSC.Server.PublishMode.Append
-        if args.replace or args.overwrite:
-            if args.append:
-                return None  # invalid combination
-            # Overwrites the workbook, data source, or data extract if it already exists on the server.
-            publish_mode = TSC.Server.PublishMode.Overwrite
-        else:
-            # default: fail if it already exists on the server
-            publish_mode = TSC.Server.PublishMode.CreateNew
+            if publish_mode != default_mode:
+                publish_mode = None
+            else:
+                # only relevant for datasources, but tsc will throw an error for us if necessary
+                publish_mode = TSC.Server.PublishMode.Append
+
+        if args.overwrite:
+            if publish_mode != default_mode:
+                publish_mode = None
+            else:
+                # Overwrites the workbook, data source, or data extract if it already exists on the server.
+                publish_mode = TSC.Server.PublishMode.Overwrite
+
+        if not publish_mode:
+            Errors.exit_with_error(logger, "Invalid combination of publishing options (Append, Overwrite, Replace)")
+        logger.debug("Publish mode selected: " + publish_mode)
         return publish_mode

From 6869a241a8b78c146af22ab446e045630fa2d824 Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Fri, 10 Mar 2023 13:26:11 -0800
Subject: [PATCH 6/7] formatting

---
 tabcmd/commands/constants.py                                 | 2 +-
 tabcmd/commands/datasources_and_workbooks/export_command.py  | 2 +-
 tabcmd/commands/datasources_and_workbooks/publish_command.py | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tabcmd/commands/constants.py b/tabcmd/commands/constants.py
index bf633560..77fe6198 100644
--- a/tabcmd/commands/constants.py
+++ b/tabcmd/commands/constants.py
@@ -56,7 +56,7 @@ def log_stack(logger):
             logger.info("Error printing stack trace:", e)
 
     @staticmethod
-    def exit_with_error(logger, message=None, exception:Exception=None):
+    def exit_with_error(logger, message=None, exception: Exception = None):
         try:
             if message and not exception:
                 logger.error(message)
diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py
index 4ee355e0..b64e9c6f 100644
--- a/tabcmd/commands/datasources_and_workbooks/export_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/export_command.py
@@ -54,7 +54,7 @@ def define_args(export_parser):
             help="Set the page size of the exported PDF",
         )
 
-        group.add_argument("--width", default=800, help="Set the width of the image in pixels. Default is 800 px")
+        group.add_argument("--width", default=800, help="[Not Yet Implemented] Set the width of the image in pixels. Default is 800 px")
         group.add_argument("--filename", "-f", help="filename to store the exported data")
         group.add_argument("--height", default=600, help=_("export.options.height"))
         group.add_argument(
diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py
index 2df9927e..5aaf7db2 100644
--- a/tabcmd/commands/datasources_and_workbooks/publish_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py
@@ -101,7 +101,6 @@ def run_command(args):
                 Errors.exit_with_error(logger, exception=exc)
             logger.info(_("publish.success") + "\n{}".format(new_datasource.webpage_url))
 
-
     # todo write tests for this method
     @staticmethod
     def get_publish_mode(args, logger):

From 248b08d0070ce444e36f649116923365fd0839de Mon Sep 17 00:00:00 2001
From: Jac Fitzgerald <jac.fitzgerald@salesforce.com>
Date: Fri, 10 Mar 2023 13:29:23 -0800
Subject: [PATCH 7/7] mypy

hmm, this didn't complain locally.
---
 tabcmd/commands/auth/session.py                             | 2 +-
 tabcmd/commands/constants.py                                | 3 ++-
 .../datasources_and_workbooks_command.py                    | 6 +++---
 tabcmd/commands/datasources_and_workbooks/export_command.py | 4 +++-
 tabcmd/commands/user/user_data.py                           | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tabcmd/commands/auth/session.py b/tabcmd/commands/auth/session.py
index 2e7107ef..1dcf34a7 100644
--- a/tabcmd/commands/auth/session.py
+++ b/tabcmd/commands/auth/session.py
@@ -249,7 +249,7 @@ def _sign_in(self, tableau_auth) -> TSC.Server:
                 self.username = self.tableau_server.users.get_by_id(self.user_id).name
             self.logger.info(_("common.output.succeeded"))
         except Exception as e:
-            Errors.exit_with_error(self.logger, e)
+            Errors.exit_with_error(self.logger, exception=e)
         self.logger.debug("Signed into {0}{1} as {2}".format(self.server_url, self.site_name, self.username))
         return self.tableau_server
 
diff --git a/tabcmd/commands/constants.py b/tabcmd/commands/constants.py
index 77fe6198..263fb763 100644
--- a/tabcmd/commands/constants.py
+++ b/tabcmd/commands/constants.py
@@ -1,5 +1,6 @@
 import inspect
 import sys
+from typing import Optional
 
 import tableauserverclient
 
@@ -56,7 +57,7 @@ def log_stack(logger):
             logger.info("Error printing stack trace:", e)
 
     @staticmethod
-    def exit_with_error(logger, message=None, exception: Exception = None):
+    def exit_with_error(logger, message: Optional[str] = None, exception: Optional[Exception] = None):
         try:
             if message and not exception:
                 logger.error(message)
diff --git a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py
index 1a2169f5..fb0d5daf 100644
--- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py
@@ -28,7 +28,7 @@ def get_view_by_content_url(logger, server, view_content_url) -> TSC.ViewItem:
             logger.debug(req_option.get_query_params())
             matching_views, paging = server.views.get(req_option)
         except Exception as e:
-            Errors.exit_with_error(logger, e)
+            Errors.exit_with_error(logger, exception=e)
         if len(matching_views) < 1:
             Errors.exit_with_error(logger, message=_("errors.xmlapi.not_found"))
         return matching_views[0]
@@ -42,7 +42,7 @@ def get_wb_by_content_url(logger, server, workbook_content_url) -> TSC.WorkbookI
             logger.debug(req_option.get_query_params())
             matching_workbooks, paging = server.workbooks.get(req_option)
         except Exception as e:
-            Errors.exit_with_error(logger, e)
+            Errors.exit_with_error(logger, exception=e)
         if len(matching_workbooks) < 1:
             Errors.exit_with_error(logger, message=_("dataalerts.failure.error.workbookNotFound"))
         return matching_workbooks[0]
@@ -56,7 +56,7 @@ def get_ds_by_content_url(logger, server, datasource_content_url) -> TSC.Datasou
             logger.debug(req_option.get_query_params())
             matching_datasources, paging = server.datasources.get(req_option)
         except Exception as e:
-            Errors.exit_with_error(logger, e)
+            Errors.exit_with_error(logger, exception=e)
         if len(matching_datasources) < 1:
             Errors.exit_with_error(logger, message=_("dataalerts.failure.error.datasourceNotFound"))
         return matching_datasources[0]
diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py
index b64e9c6f..3ecd741b 100644
--- a/tabcmd/commands/datasources_and_workbooks/export_command.py
+++ b/tabcmd/commands/datasources_and_workbooks/export_command.py
@@ -54,7 +54,9 @@ def define_args(export_parser):
             help="Set the page size of the exported PDF",
         )
 
-        group.add_argument("--width", default=800, help="[Not Yet Implemented] Set the width of the image in pixels. Default is 800 px")
+        group.add_argument(
+            "--width", default=800, help="[Not Yet Implemented] Set the width of the image in pixels. Default is 800 px"
+        )
         group.add_argument("--filename", "-f", help="filename to store the exported data")
         group.add_argument("--height", default=600, help=_("export.options.height"))
         group.add_argument(
diff --git a/tabcmd/commands/user/user_data.py b/tabcmd/commands/user/user_data.py
index b9012078..5251aba4 100644
--- a/tabcmd/commands/user/user_data.py
+++ b/tabcmd/commands/user/user_data.py
@@ -260,7 +260,7 @@ def act_on_users(
         try:
             group = UserCommand.find_group(logger, server, args.name)
         except Exception as e:
-            Errors.exit_with_error(logger, e)
+            Errors.exit_with_error(logger, exception=e)
 
         n_users_handled: int = 0
         number_of_errors: int = 0