Common error for mutually exclusive flags (#1375)

* Add exception and OverridableParamete * Add incompatible error message to app events command * Update error message to be more understandable * Add ignore type flag * Update test name * Remove unused imports
snowflakedb · Aug 14, 2024 · 7ed0a38 · 7ed0a38
1 parent 53c31d7
commit 7ed0a38
Show file tree

Hide file tree

Showing 21 changed files with 342 additions and 206 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -36,6 +36,8 @@
 * Fixed problem with whitespaces in `snow connection add` command
 * Added check for the correctness of token file and private key paths when addind a connection
 * Fix the typo in spcs service name argument description. It is the identifier of the **service** instead of the **service pool**.
+* Improved error message for incompatible parameters.
+
 
 # v2.7.0
 

diff --git a/src/snowflake/cli/_plugins/cortex/commands.py b/src/snowflake/cli/_plugins/cortex/commands.py
@@ -31,7 +31,10 @@
     Text,
 )
 from snowflake.cli.api.cli_global_context import get_cli_context
-from snowflake.cli.api.commands.flags import readable_file_option
+from snowflake.cli.api.commands.overrideable_parameter import (
+    OverrideableArgument,
+    OverrideableOption,
+)
 from snowflake.cli.api.commands.snow_typer import SnowTyperFactory
 from snowflake.cli.api.constants import PYTHON_3_12
 from snowflake.cli.api.output.types import (
@@ -48,6 +51,25 @@
 
 SEARCH_COMMAND_ENABLED = sys.version_info < PYTHON_3_12
 
+SOURCE_EXCLUSIVE_OPTION_NAMES = ["text", "file", "source_document_text"]
+
+# Creates a Typer option and verifies if the mutually exclusive options are set in the command.
+ExclusiveReadableFileOption = OverrideableOption(
+    None,
+    "--file",
+    mutually_exclusive=SOURCE_EXCLUSIVE_OPTION_NAMES,
+    exists=True,
+    file_okay=True,
+    dir_okay=False,
+    readable=True,
+    show_default=False,
+)
+
+# Creates a Typer argument and verifies if the mutually exclusive options are set in the command.
+ExclusiveTextSourceArgument = OverrideableArgument(
+    mutually_exclusive=SOURCE_EXCLUSIVE_OPTION_NAMES,
+)
+
 
 @app.command(
     requires_connection=True,
@@ -100,8 +122,8 @@ def search(
     requires_connection=True,
 )
 def complete(
-    text: Optional[str] = typer.Argument(
-        None,
+    text: Optional[str] = ExclusiveTextSourceArgument(
+        default=None,
         help="Prompt to be used to generate a completion. Cannot be combined with --file option.",
         show_default=False,
     ),
@@ -110,9 +132,8 @@ def complete(
         "--model",
         help="String specifying the model to be used.",
     ),
-    file: Optional[Path] = readable_file_option(
-        param_name="--file",
-        help_str="JSON file containing conversation history to be used to generate a completion. Cannot be combined with TEXT argument.",
+    file: Optional[Path] = ExclusiveReadableFileOption(
+        help="JSON file containing conversation history to be used to generate a completion. Cannot be combined with TEXT argument.",
     ),
     **options,
 ) -> CommandResult:
@@ -124,8 +145,6 @@ def complete(
 
     manager = CortexManager()
 
-    if text and file:
-        raise UsageError("--file option cannot be used together with TEXT argument.")
     if text:
         result_text = manager.complete_for_prompt(
             text=Text(text),
@@ -152,14 +171,13 @@ def extract_answer(
         help="String containing the question to be answered.",
         show_default=False,
     ),
-    source_document_text: Optional[str] = typer.Argument(
-        None,
+    source_document_text: Optional[str] = ExclusiveTextSourceArgument(
+        default=None,
         help="String containing the plain-text or JSON document that contains the answer to the question. Cannot be combined with --file option.",
         show_default=False,
     ),
-    file: Optional[Path] = readable_file_option(
-        param_name="--file",
-        help_str="File containing the plain-text or JSON document that contains the answer to the question. Cannot be combined with SOURCE_DOCUMENT_TEXT argument.",
+    file: Optional[Path] = ExclusiveReadableFileOption(
+        help="File containing the plain-text or JSON document that contains the answer to the question. Cannot be combined with SOURCE_DOCUMENT_TEXT argument.",
     ),
     **options,
 ) -> CommandResult:
@@ -170,10 +188,6 @@ def extract_answer(
 
     manager = CortexManager()
 
-    if source_document_text and file:
-        raise UsageError(
-            "--file option cannot be used together with SOURCE_DOCUMENT_TEXT argument."
-        )
     if source_document_text:
         result_text = manager.extract_answer_from_source_document(
             source_document=SourceDocument(source_document_text),
@@ -197,14 +211,13 @@ def extract_answer(
     requires_connection=True,
 )
 def sentiment(
-    text: Optional[str] = typer.Argument(
-        None,
+    text: Optional[str] = ExclusiveTextSourceArgument(
+        default=None,
         help="String containing the text for which a sentiment score should be calculated. Cannot be combined with --file option.",
         show_default=False,
     ),
-    file: Optional[Path] = readable_file_option(
-        param_name="--file",
-        help_str="File containing the text for which a sentiment score should be calculated. Cannot be combined with TEXT argument.",
+    file: Optional[Path] = ExclusiveReadableFileOption(
+        help="File containing the text for which a sentiment score should be calculated. Cannot be combined with TEXT argument.",
     ),
     **options,
 ) -> CommandResult:
@@ -216,8 +229,6 @@ def sentiment(
 
     manager = CortexManager()
 
-    if text and file:
-        raise UsageError("--file option cannot be used together with TEXT argument.")
     if text:
         result_text = manager.calculate_sentiment_for_text(
             text=Text(text),
@@ -237,14 +248,13 @@ def sentiment(
     requires_connection=True,
 )
 def summarize(
-    text: Optional[str] = typer.Argument(
-        None,
+    text: Optional[str] = ExclusiveTextSourceArgument(
+        default=None,
         help="String containing the English text from which a summary should be generated. Cannot be combined with --file option.",
         show_default=False,
     ),
-    file: Optional[Path] = readable_file_option(
-        param_name="--file",
-        help_str="File containing the English text from which a summary should be generated. Cannot be combined with TEXT argument.",
+    file: Optional[Path] = ExclusiveReadableFileOption(
+        help="File containing the English text from which a summary should be generated. Cannot be combined with TEXT argument.",
     ),
     **options,
 ) -> CommandResult:
@@ -254,8 +264,6 @@ def summarize(
 
     manager = CortexManager()
 
-    if text and file:
-        raise UsageError("--file option cannot be used together with TEXT argument.")
     if text:
         result_text = manager.summarize_text(
             text=Text(text),
@@ -275,8 +283,8 @@ def summarize(
     requires_connection=True,
 )
 def translate(
-    text: Optional[str] = typer.Argument(
-        None,
+    text: Optional[str] = ExclusiveTextSourceArgument(
+        default=None,
         help="String containing the text to be translated. Cannot be combined with --file option.",
         show_default=False,
     ),
@@ -292,9 +300,8 @@ def translate(
         help="String specifying the language code into which the text should be translated. See Snowflake Cortex documentation for a list of supported language codes.",
         show_default=False,
     ),
-    file: Optional[Path] = readable_file_option(
-        param_name="--file",
-        help_str="File containing the text to be translated. Cannot be combined with TEXT argument.",
+    file: Optional[Path] = ExclusiveReadableFileOption(
+        help="File containing the text to be translated. Cannot be combined with TEXT argument.",
     ),
     **options,
 ) -> CommandResult:
@@ -307,8 +314,6 @@ def translate(
     source_language = None if from_language is None else Language(from_language)
     target_language = Language(to_language)
 
-    if text and file:
-        raise UsageError("--file option cannot be used together with TEXT argument.")
     if text:
         result_text = manager.translate_text(
             text=Text(text),

diff --git a/src/snowflake/cli/_plugins/nativeapp/commands.py b/src/snowflake/cli/_plugins/nativeapp/commands.py
@@ -22,7 +22,6 @@
 from typing import Generator, Iterable, List, Optional, cast
 
 import typer
-from click import ClickException, UsageError
 from snowflake.cli._plugins.nativeapp.common_flags import (
     ForceOption,
     InteractiveOption,
@@ -60,6 +59,7 @@
     with_project_definition,
 )
 from snowflake.cli.api.commands.snow_typer import SnowTyperFactory
+from snowflake.cli.api.exceptions import IncompatibleParametersError
 from snowflake.cli.api.output.formats import OutputFormat
 from snowflake.cli.api.output.types import (
     CollectionResult,
@@ -371,7 +371,7 @@ def app_deploy(
             recursive = False
 
     if has_paths and prune:
-        raise ClickException("--prune cannot be used when paths are also specified")
+        raise IncompatibleParametersError(["paths", "--prune"])
 
     cli_context = get_cli_context()
     manager = NativeAppManager(
@@ -495,18 +495,18 @@ def app_events(
     https://docs.snowflake.com/en/developer-guide/native-apps/setting-up-logging-and-events
     """
     if first >= 0 and last >= 0:
-        raise UsageError("--first and --last cannot be used together.")
+        raise IncompatibleParametersError(["--first", "--last"])
 
     if (consumer_org and not consumer_account) or (
         consumer_account and not consumer_org
     ):
-        raise UsageError("--consumer-org and --consumer-account must be used together.")
+        raise IncompatibleParametersError(["--consumer-org", "--consumer-account"])
 
     if follow:
         if until:
-            raise UsageError("--follow and --until cannot be used together.")
+            raise IncompatibleParametersError(["--follow", "--until"])
         if first >= 0:
-            raise UsageError("--follow and --first cannot be used together.")
+            raise IncompatibleParametersError(["--follow", "--first"])
 
     assert_project_type("native_app")
 

diff --git a/src/snowflake/cli/_plugins/object/commands.py b/src/snowflake/cli/_plugins/object/commands.py
@@ -26,6 +26,7 @@
 )
 from snowflake.cli.api.commands.snow_typer import SnowTyperFactory
 from snowflake.cli.api.constants import SUPPORTED_OBJECTS, VALID_SCOPES
+from snowflake.cli.api.exceptions import IncompatibleParametersError
 from snowflake.cli.api.identifiers import FQN
 from snowflake.cli.api.output.types import MessageResult, QueryResult
 from snowflake.cli.api.project.util import is_valid_identifier
@@ -153,9 +154,7 @@ def create(
     import json
 
     if object_attributes and object_json:
-        raise ClickException(
-            "Conflict: both object attributes and JSON definition are provided"
-        )
+        raise IncompatibleParametersError(["object_attributes", "--json"])
 
     if object_json:
         object_data = json.loads(object_json)

diff --git a/src/snowflake/cli/_plugins/sql/commands.py b/src/snowflake/cli/_plugins/sql/commands.py
@@ -17,43 +17,46 @@
 from pathlib import Path
 from typing import List, Optional
 
-import typer
 from snowflake.cli._plugins.sql.manager import SqlManager
 from snowflake.cli.api.commands.decorators import with_project_definition
 from snowflake.cli.api.commands.flags import (
     parse_key_value_variables,
     variables_option,
 )
+from snowflake.cli.api.commands.overrideable_parameter import OverrideableOption
 from snowflake.cli.api.commands.snow_typer import SnowTyperFactory
 from snowflake.cli.api.output.types import CommandResult, MultipleResults, QueryResult
 
 # simple Typer with defaults because it won't become a command group as it contains only one command
 app = SnowTyperFactory()
 
+SOURCE_EXCLUSIVE_OPTIONS_NAMES = ["query", "files", "std_in"]
+
+SourceOption = OverrideableOption(
+    mutually_exclusive=SOURCE_EXCLUSIVE_OPTIONS_NAMES, show_default=False
+)
+
 
 @app.command(name="sql", requires_connection=True, no_args_is_help=True)
 @with_project_definition(is_optional=True)
 def execute_sql(
-    query: Optional[str] = typer.Option(
-        None,
-        "--query",
-        "-q",
+    query: Optional[str] = SourceOption(
+        default=None,
+        param_decls=["--query", "-q"],
         help="Query to execute.",
     ),
-    files: Optional[List[Path]] = typer.Option(
-        None,
-        "--filename",
-        "-f",
+    files: Optional[List[Path]] = SourceOption(
+        default=[],
+        param_decls=["--filename", "-f"],
         exists=True,
         file_okay=True,
         dir_okay=False,
         readable=True,
         help="File to execute.",
     ),
-    std_in: Optional[bool] = typer.Option(
-        False,
-        "--stdin",
-        "-i",
+    std_in: Optional[bool] = SourceOption(
+        default=False,
+        param_decls=["--stdin", "-i"],
         help="Read the query from standard input. Use it when piping input to this command.",
     ),
     data_override: List[str] = variables_option(
@@ -73,6 +76,7 @@ def execute_sql(
     The command supports variable substitution that happens on client-side. Both &VARIABLE or &{ VARIABLE }
     syntax are supported.
     """
+
     data = {}
     if data_override:
         data = {v.key: v.value for v in parse_key_value_variables(data_override)}