Snow 1533042 create error handling fix

RELEASE-NOTES.md

@@ -33,6 +33,15 @@
 * Added support for user stages in stage and git copy commands
 * Improved support for quoted identifiers in snowpark commands.
 
+# v2.6.1
+## Backward incompatibility
+
+## Deprecations
+
+## New additions
+
+## Fixes and improvements
+* `snow object create` message returns meaningful error if connection database is not defined.
 
 # v2.6.0
 ## Backward incompatibility

src/snowflake/cli/api/rest_api.py

@@ -16,16 +16,27 @@
 
 import json
 import logging
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, Optional
 
+from click import ClickException
 from snowflake.cli.api.constants import SF_REST_API_URL_PREFIX
 from snowflake.connector.connection import SnowflakeConnection
-from snowflake.connector.errors import InterfaceError
+from snowflake.connector.errors import BadRequest, InterfaceError
 from snowflake.connector.network import SnowflakeRestful
 
 log = logging.getLogger(__name__)
 
 
+def _pluralize_object_type(object_type: str) -> str:
+    """
+    Pluralize object type without depending on OBJECT_TO_NAMES.
+    """
+    if object_type.endswith("y"):
+        return object_type[:-1].lower() + "ies"
+    else:
+        return object_type.lower() + "s"
+
+
 class RestApi:
     def __init__(self, connection: SnowflakeConnection):
         self.conn = connection
@@ -43,6 +54,13 @@ def get_endpoint_exists(self, url: str) -> bool:
                 return False
             raise err
 
+    def _fetch_endpoint_exists(self, url: str) -> bool:
+        try:
+            result = self.send_rest_request(url, method="get")
+            return bool(result)
+        except BadRequest:
+            return False
+
     def send_rest_request(
         self, url: str, method: str, data: Optional[Dict[str, Any]] = None
     ):
@@ -75,12 +93,18 @@ def send_rest_request(
             no_retry=True,
         )
 
-    def determine_url_for_create_query(
-        self, *, plural_object_type: str
-    ) -> Optional[str]:
+    def _database_exists(self, db_name: str) -> bool:
+        url = f"{SF_REST_API_URL_PREFIX}/databases/{db_name}"
+        return self._fetch_endpoint_exists(url)
+
+    def _schema_exists(self, db_name: str, schema_name: str) -> bool:
+        url = f"{SF_REST_API_URL_PREFIX}/databases/{db_name}/schemas/{schema_name}"
+        return self._fetch_endpoint_exists(url)
+
+    def determine_url_for_create_query(self, object_type: str) -> str:
         """
         Determine an url for creating an object of given type via REST API.
-        The function returns None if URL cannot be determined.
+        If URL cannot be determined, the function throws CannotDetermineCreateURLException exception.
 
         URLs we check:
          * /api/v2/<type>/
@@ -92,22 +116,57 @@ def determine_url_for_create_query(
          To check whether an URL exists, we send read-only GET request (LIST endpoint,
          which should imply CREATE endpoint).
         """
-        urls_to_be_checked: List[Optional[str]] = [
-            f"{SF_REST_API_URL_PREFIX}/{plural_object_type}/",
-            (
-                f"{SF_REST_API_URL_PREFIX}/databases/{self.conn.database}/{plural_object_type}/"
-                if self.conn.database
-                else None
-            ),
-            (
-                f"{SF_REST_API_URL_PREFIX}/databases/{self.conn.database}/schemas/{self.conn.schema}/{plural_object_type}/"
-                if self.conn.database and self.conn.schema
-                else None
-            ),
-        ]
-
-        for url in urls_to_be_checked:
-            if url and self.get_endpoint_exists(url):
-                return url
-
-        return None
+        plural_object_type = _pluralize_object_type(object_type)
+
+        if self.get_endpoint_exists(
+            url := f"{SF_REST_API_URL_PREFIX}/{plural_object_type}/"
+        ):
+            return url
+
+        db = self.conn.database
+        if not db:
+            raise DatabaseNotDefinedException(
+                "Database not defined in connection. Please try again with `--database` flag."
+            )
+        if not self._database_exists(db):
+            raise DatabaseNotExistsException(f"Database '{db}' does not exist.")
+        if self.get_endpoint_exists(
+            url := f"{SF_REST_API_URL_PREFIX}/databases/{db}/{plural_object_type}/"
+        ):
+            return url
+
+        schema = self.conn.schema
+        if not schema:
+            raise SchemaNotDefinedException(
+                "Schema not defined in connection. Please try again with `--schema` flag."
+            )
+        if not self._schema_exists(db_name=db, schema_name=schema):
+            raise SchemaNotExistsException(f"Schema '{schema}' does not exist.")
+        if self.get_endpoint_exists(
+            url := f"{SF_REST_API_URL_PREFIX}/databases/{self.conn.database}/schemas/{self.conn.schema}/{plural_object_type}/"
+        ):
+            return url
+
+        raise CannotDetermineCreateURLException(
+            f"Create operation for type {object_type} is not supported. Try using `sql -q 'CREATE ...'` command."
+        )
+
+
+class DatabaseNotDefinedException(ClickException):
+    pass
+
+
+class SchemaNotDefinedException(ClickException):
+    pass
+
+
+class DatabaseNotExistsException(ClickException):
+    pass
+
+
+class SchemaNotExistsException(ClickException):
+    pass
+
+
+class CannotDetermineCreateURLException(ClickException):
+    pass

src/snowflake/cli/plugins/object/manager.py

@@ -36,16 +36,6 @@ def _get_object_names(object_type: str) -> ObjectNames:
     return OBJECT_TO_NAMES[object_type]
 
 
-def _pluralize_object_type(object_type: str) -> str:
-    """
-    Pluralize object type without depending on OBJECT_TO_NAMES.
-    """
-    if object_type.endswith("y"):
-        return object_type[:-1].lower() + "ies"
-    else:
-        return object_type.lower() + "s"
-
-
 class ObjectManager(SqlExecutionMixin):
     def show(
         self,
@@ -85,11 +75,8 @@ def object_exists(self, *, object_type: str, name: str):
 
     def create(self, object_type: str, object_data: Dict[str, Any]) -> str:
         rest = RestApi(self._conn)
-        url = rest.determine_url_for_create_query(
-            plural_object_type=_pluralize_object_type(object_type)
-        )
-        if not url:
-            return f"Create operation for type {object_type} is not supported. Try using `sql -q 'CREATE ...'` command"
+        url = rest.determine_url_for_create_query(object_type=object_type)
+
         try:
             response = rest.send_rest_request(url=url, method="post", data=object_data)
             # workaround as SnowflakeRestful class ignores some errors, dropping their info and returns {} instead.

tests/api/test_rest_api.py

@@ -18,7 +18,7 @@
 from unittest import mock
 
 import pytest
-from snowflake.cli.api.rest_api import RestApi
+from snowflake.cli.api.rest_api import CannotDetermineCreateURLException, RestApi
 from snowflake.connector.errors import InterfaceError
 
 _DUMMY_SERVER_URL = "https://DUMMY_SERVER_URL"
@@ -106,15 +106,20 @@ def test_determine_create_url(mock_rest_connection, number_of_fails):
 
     a_type = "a_type"
     urls = [
-        f"/api/v2/{a_type}/",
-        f"/api/v2/databases/DB/{a_type}/",
-        f"/api/v2/databases/DB/schemas/SCHEMA/{a_type}/",
-        None,
+        f"/api/v2/{a_type}s/",
+        f"/api/v2/databases/DB/{a_type}s/",
+        f"/api/v2/databases/DB/schemas/SCHEMA/{a_type}s/",
     ]
 
     rest = RestApi(mock_rest_connection.connection)
-    result = rest.determine_url_for_create_query(plural_object_type=a_type)
-    assert result == urls[number_of_fails]
+    # mock additional check
+    rest._fetch_endpoint_exists = lambda _: True  # noqa: SLF001
+
+    try:
+        result = rest.determine_url_for_create_query(a_type)
+        assert result == urls[number_of_fails]
+    except CannotDetermineCreateURLException:
+        assert number_of_fails == 3
 
     mock_rest_connection.assert_rest_fetch_calls_matches(
         [

tests_integration/test_object.py

@@ -149,6 +149,20 @@ def test_show_drop_image_repository(runner, test_database, snowflake_session):
             },
         ),
         ("image-repository", {"name": "test_image_repo"}),
+        (
+            "table",
+            {
+                "name": "test_table",
+                "columns": [{"name": "col1", "datatype": "number", "nullable": False}],
+                "constraints": [
+                    {
+                        "name": "prim_key",
+                        "column_names": ["col1"],
+                        "constraint_type": "PRIMARY KEY",
+                    }
+                ],
+            },
+        ),
     ],
 )
 @pytest.mark.integration
@@ -185,13 +199,14 @@ def _test_create(params):
         _test_create(["--json", json.dumps(object_definition)])
     # test key=value format
     with _cleanup_object():
-        list_definition = [f"{key}={value}" for key, value in object_definition.items()]
+        list_definition = [
+            f"{key}={json.dumps(value)}" for key, value in object_definition.items()
+        ]
         _test_create(list_definition)
 
 
 @pytest.mark.integration
-def test_create_errors(runner, test_database, caplog):
-
+def test_create_error_conflict(runner, test_database, caplog):
     # conflict - an object already exists
     schema_name = "schema_noble_knight"
     result = runner.invoke_with_connection(
@@ -207,6 +222,9 @@ def test_create_errors(runner, test_database, caplog):
     assert "409 Conflict" in caplog.text
     caplog.clear()
 
+
+@pytest.mark.integration
+def test_create_error_misspelled_argument(runner, test_database, caplog):
     # misspelled argument
     schema_name = "another_schema_name"
     result = runner.invoke_with_connection(
@@ -220,13 +238,67 @@ def test_create_errors(runner, test_database, caplog):
     assert "HTTP 400: Bad Request" in caplog.text
     caplog.clear()
 
+
+@pytest.mark.integration
+def test_create_error_unsupported_type(runner, test_database):
     # object type that don't exist
     result = runner.invoke_with_connection(
-        ["object", "create", "type_that_does_not_exist", "name=anything", "--debug"]
+        ["object", "create", "type_that_does_not_exist", "name=anything"]
+    )
+    assert result.exit_code == 1
+    assert "Error" in result.output
+    assert (
+        "Create operation for type type_that_does_not_exist is not supported."
+        in result.output
+    )
+    assert "using `sql -q 'CREATE ...'` command." in result.output
+
+
+@pytest.mark.integration
+def test_create_error_database_not_exist(runner):
+    # database does not exist
+    result = runner.invoke_with_connection(
+        [
+            "object",
+            "create",
+            "schema",
+            "name=test_schema",
+            "--database",
+            "this_db_does_not_exist",
+        ]
+    )
+    assert result.exit_code == 1, result.output
+    assert "Error" in result.output
+    assert "Database 'THIS_DB_DOES_NOT_EXIST' does not exist." in result.output
+
+
+@pytest.mark.integration
+def test_create_error_schema_not_exist(runner, test_database):
+    # schema does not exist
+    result = runner.invoke_with_connection(
+        [
+            "object",
+            "create",
+            "image-repository",
+            "name=test_schema",
+            "--schema",
+            "this_schema_does_not_exist",
+        ]
+    )
+    assert result.exit_code == 1, result.output
+    assert "Error" in result.output
+    assert "Schema 'THIS_SCHEMA_DOES_NOT_EXIST' does not exist." in result.output
+
+
+@pytest.mark.integration
+def test_create_error_undefined_database(runner):
+    # undefined database
+    result = runner.invoke_with_connection(
+        ["object", "create", "schema", f"name=test_schema"]
     )
-    assert result.exit_code == 0
+    assert result.exit_code == 1, result.output
+    assert "Error" in result.output
     assert (
-        "Create operation for type type_that_does_not_exist is not supported. Try using `sql -q 'CREATE ...'` command"
+        "Database not defined in connection. Please try again with `--database` flag."
         in result.output
     )
-    assert "404 Not Found" in caplog.text