Skip to content

Commit

Permalink
[SNOW-1486485] Fix tearing down an app whose package has already been…
Browse files Browse the repository at this point in the history
… dropped (#1224)

When a package is dropped, subsequent `snow app teardown` calls fail with “Application is no longer available for use. Please contact the application provider for more details.” since the CLI is still trying to execute `show objects owned by application`. If this query fails with this error, we should ask the user if they want to cascade drop the application without being able to see the objects.
  • Loading branch information
sfc-gh-fcampbell authored Jul 2, 2024
1 parent 2bc8245 commit 8cb7228
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 16 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
## Fixes and improvements
* Passing a directory to `snow app deploy` will now deploy any contained file or subfolder specified in the application's artifact rules
* Fixes markup escaping errors in `snow sql` that may occur when users use unintentionally markup-like escape tags.
* Fixed case where `snow app teardown` could not tear down orphan applications (those that have had their package dropped)
* Fixed case where `snow app teardown` could leave behind orphan applications if they were not created by the Snowflake CLI
* Fixed case where `snow app run` could fail to run an existing application whose package was dropped by prompting to drop and recreate the application
* Improve terminal output sanitization to avoid ASCII escape codes.
Expand Down
15 changes: 15 additions & 0 deletions src/snowflake/cli/plugins/nativeapp/errno.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright (c) 2024 Snowflake Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

APPLICATION_NO_LONGER_AVAILABLE = 93079
5 changes: 4 additions & 1 deletion src/snowflake/cli/plugins/nativeapp/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,12 @@ def _application_objects_to_str(
...
"""
return "\n".join(
[f"({obj['type']}) {obj['name']}" for obj in application_objects]
[self._application_object_to_str(obj) for obj in application_objects]
)

def _application_object_to_str(self, obj: ApplicationOwnedObject) -> str:
return f"({obj['type']}) {obj['name']}"

def get_snowsight_url(self) -> str:
"""Returns the URL that can be used to visit this app via Snowsight."""
name = identifier_for_url(self.app_name)
Expand Down
69 changes: 55 additions & 14 deletions src/snowflake/cli/plugins/nativeapp/teardown_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
INTERNAL_DISTRIBUTION,
OWNER_COL,
)
from snowflake.cli.plugins.nativeapp.errno import APPLICATION_NO_LONGER_AVAILABLE
from snowflake.cli.plugins.nativeapp.exceptions import (
CouldNotDropApplicationPackageWithVersions,
)
Expand All @@ -39,6 +40,7 @@
from snowflake.cli.plugins.nativeapp.utils import (
needs_confirmation,
)
from snowflake.connector import ProgrammingError
from snowflake.connector.cursor import DictCursor


Expand Down Expand Up @@ -114,22 +116,57 @@ def drop_application(
raise typer.Abort()

# 4. Check for application objects owned by the application
application_objects = self.get_objects_owned_by_application()
if len(application_objects) > 0:
application_objects_str = self._application_objects_to_str(
application_objects
# This query will fail if the application package has already been dropped, so handle this case gracefully
has_objects_to_drop = False
message_prefix = ""
cascade_true_message = ""
cascade_false_message = ""
interactive_prompt = ""
non_interactive_abort = ""
try:
if application_objects := self.get_objects_owned_by_application():
has_objects_to_drop = True
message_prefix = (
f"The following objects are owned by application {self.app_name}"
)
cascade_true_message = f"{message_prefix} and will be dropped:"
cascade_false_message = f"{message_prefix} and will NOT be dropped:"
interactive_prompt = "Would you like to drop these objects in addition to the application? [y/n/ABORT]"
non_interactive_abort = "Re-run teardown again with --cascade or --no-cascade to specify whether these objects should be dropped along with the application"
except ProgrammingError as e:
if e.errno != APPLICATION_NO_LONGER_AVAILABLE:
raise
application_objects = []
message_prefix = f"Could not determine which objects are owned by application {self.app_name}"
has_objects_to_drop = True # potentially, but we don't know what they are
cascade_true_message = (
f"{message_prefix}, an unknown number of objects will be dropped."
)
cascade_false_message = f"{message_prefix}, they will NOT be dropped."
interactive_prompt = f"Would you like to drop an unknown set of objects in addition to the application? [y/n/ABORT]"
non_interactive_abort = f"Re-run teardown again with --cascade or --no-cascade to specify whether any objects should be dropped along with the application."

if has_objects_to_drop:
if cascade is True:
cc.message(
f"The following objects are owned by application {self.app_name} and will be dropped:\n{application_objects_str}"
)
# If the user explicitly passed the --cascade flag
cc.message(cascade_true_message)
with cc.indented():
for obj in application_objects:
cc.message(self._application_object_to_str(obj))
elif cascade is False:
cc.message(
f"The following objects are owned by application {self.app_name}:\n{application_objects_str}"
)
# If the user explicitly passed the --no-cascade flag
cc.message(cascade_false_message)
with cc.indented():
for obj in application_objects:
cc.message(self._application_object_to_str(obj))
elif interactive:
# If the user didn't pass any cascade flag and the session is interactive
cc.message(message_prefix)
with cc.indented():
for obj in application_objects:
cc.message(self._application_object_to_str(obj))
user_response = typer.prompt(
f"The following objects are owned by application {self.app_name}:\n{application_objects_str}\n\nWould you like to drop these objects in addition to the application? [y/n/ABORT]",
interactive_prompt,
show_default=False,
default="ABORT",
).lower()
Expand All @@ -140,11 +177,15 @@ def drop_application(
else:
raise typer.Abort()
else:
cc.message(
f"The following application objects are owned by application {self.app_name}:\n{application_objects_str}\n\nRe-run teardown again with --cascade or --no-cascade to specify whether these objects should be dropped along with the application."
)
# Else abort since we don't know what to do and can't ask the user
cc.message(message_prefix)
with cc.indented():
for obj in application_objects:
cc.message(self._application_object_to_str(obj))
cc.message(non_interactive_abort)
raise typer.Abort()
elif cascade is None:
# If there's nothing to drop, set cascade to an explicit False value
cascade = False

# 5. All validations have passed, drop object
Expand Down
62 changes: 62 additions & 0 deletions tests/nativeapp/__snapshots__/test_teardown_processor.ambr
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# serializer version: 1
# name: test_drop_application_cascade[False-application_objects3-None-False]
''
# ---
# name: test_drop_application_cascade[False-application_objects4-None-False]
'''
The following objects are owned by application myapp and will NOT be dropped:
(DATABASE) db

'''
# ---
# name: test_drop_application_cascade[False-application_objects5-None-False]
'''
Could not determine which objects are owned by application myapp, they will NOT be dropped.

'''
# ---
# name: test_drop_application_cascade[None-application_objects10-yes-True]
'''
Could not determine which objects are owned by application myapp

'''
# ---
# name: test_drop_application_cascade[None-application_objects11-no-False]
'''
The following objects are owned by application myapp
(DATABASE) db

'''
# ---
# name: test_drop_application_cascade[None-application_objects12-no-False]
'''
Could not determine which objects are owned by application myapp

'''
# ---
# name: test_drop_application_cascade[None-application_objects6-None-False]
''
# ---
# name: test_drop_application_cascade[None-application_objects9-yes-True]
'''
The following objects are owned by application myapp
(DATABASE) db

'''
# ---
# name: test_drop_application_cascade[True-application_objects0-None-True]
''
# ---
# name: test_drop_application_cascade[True-application_objects1-None-True]
'''
The following objects are owned by application myapp and will be dropped:
(DATABASE) db

'''
# ---
# name: test_drop_application_cascade[True-application_objects2-None-True]
'''
Could not determine which objects are owned by application myapp, an unknown number of objects will be dropped.

'''
# ---
16 changes: 15 additions & 1 deletion tests/nativeapp/test_teardown_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
SPECIAL_COMMENT,
SPECIAL_COMMENT_OLD,
)
from snowflake.cli.plugins.nativeapp.errno import APPLICATION_NO_LONGER_AVAILABLE
from snowflake.cli.plugins.nativeapp.exceptions import (
CouldNotDropApplicationPackageWithVersions,
UnexpectedOwnerError,
Expand Down Expand Up @@ -1044,16 +1045,22 @@ def test_drop_package_idempotent(
# Cascade true
[True, [], None, True],
[True, [{"type": "DATABASE", "name": "db"}], None, True],
[True, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), None, True],
# Cascade false
[False, [], None, False],
[False, [{"type": "DATABASE", "name": "db"}], None, False],
[False, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), None, False],
# Cascade unset
[None, [], None, False],
[None, [{"type": "DATABASE", "name": "db"}], None, None],
[None, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), None, None],
# Interactive
[None, [{"type": "DATABASE", "name": "db"}], "yes", True],
[None, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), "yes", True],
[None, [{"type": "DATABASE", "name": "db"}], "no", False],
[None, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), "no", False],
[None, [{"type": "DATABASE", "name": "db"}], "abort", None],
[None, ProgrammingError(errno=APPLICATION_NO_LONGER_AVAILABLE), "abort", None],
],
)
def test_drop_application_cascade(
Expand All @@ -1067,8 +1074,13 @@ def test_drop_application_cascade(
interactive_response,
expected_cascade,
temp_dir,
capsys,
snapshot,
):
mock_get_objects_owned_by_application.return_value = application_objects
if isinstance(application_objects, Exception):
mock_get_objects_owned_by_application.side_effect = application_objects
else:
mock_get_objects_owned_by_application.return_value = application_objects
mock_get_existing_app_info.return_value = {
"name": "myapp",
"owner": "app_role",
Expand All @@ -1095,3 +1107,5 @@ def test_drop_application_cascade(
role="app_role",
cascade=expected_cascade,
)
stdout, _ = capsys.readouterr()
assert stdout == snapshot
19 changes: 19 additions & 0 deletions tests_integration/nativeapp/test_teardown.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@
["app teardown", "Aborted"],
],
)
@pytest.mark.parametrize("orphan_app", [True, False])
def test_nativeapp_teardown_cascade(
command,
expected_error,
orphan_app,
runner,
snowflake_session,
temporary_working_directory,
Expand Down Expand Up @@ -117,6 +119,23 @@ def test_nativeapp_teardown_cascade(
dict(name=db_name, owner=app_name),
)

if orphan_app:
# orphan the app by dropping the application package,
# this causes future `show objects owned by application` queries to fail
# and `snow app teardown` needs to be resilient against this
package_name = f"{project_name}_pkg_{USER_NAME}".upper()
snowflake_session.execute_string(
f"drop application package {package_name}"
)
assert not_contains_row_with(
row_from_snowflake_session(
snowflake_session.execute_string(
f"show application packages like '{package_name}'",
)
),
dict(name=package_name),
)

# Run the teardown command
result = runner.invoke_with_connection_json(
command.split(),
Expand Down

0 comments on commit 8cb7228

Please sign in to comment.