Skip to content

Commit

Permalink
Avoid plain print, make sure silent is eager flag (#871)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-turbaszek authored Mar 6, 2024
1 parent 64b637e commit ac5bf54
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 25 deletions.
29 changes: 29 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,32 @@ repos:
hooks:
- id: mypy
additional_dependencies: [types-all]
- repo: local
hooks:
- id: check-print-in-code
language: pygrep
name: "Check for print statements"
entry: "print\\(|echo\\("
pass_filenames: true
files: ^src/snowflake/.*\.py$
exclude: >
(?x)
^src/snowflake/cli/api/console/.*$|
^src/snowflake/cli/app/printing.py$|
^src/snowflake/cli/app/dev/.*$|
^src/snowflake/cli/templates/.*$|
^src/snowflake/cli/api/utils/rendering.py$|
^src/snowflake/cli/plugins/spcs/common.py$
- id: check-app-imports-in-api
language: pygrep
name: "No top level cli.app imports in cli.api"
entry: "^from snowflake\\.cli\\.app"
pass_filenames: true
files: ^src/snowflake/cli/api/.*\.py$
- id: avoid-snowcli
language: pygrep
name: "Prefer snowflake CLI over snowcli"
entry: "snowcli"
pass_filenames: true
files: ^src/.*\.py$
exclude: ^src/snowflake/cli/app/telemetry.py$
4 changes: 3 additions & 1 deletion src/snowflake/cli/api/commands/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import typer
from click import ClickException
from snowflake.cli.api.cli_global_context import cli_context_manager
from snowflake.cli.api.console import cli_console
from snowflake.cli.api.output.formats import OutputFormat

DEFAULT_CONTEXT_SETTINGS = {"help_option_names": ["--help", "-h"]}
Expand Down Expand Up @@ -172,7 +173,7 @@ def callback(value):

def _password_callback(value: str):
if value:
click.echo(PLAIN_PASSWORD_MSG)
cli_console.message(PLAIN_PASSWORD_MSG)

return _callback(lambda: cli_context_manager.connection_context.set_password)(value)

Expand Down Expand Up @@ -280,6 +281,7 @@ def _password_callback(value: str):
callback=_callback(lambda: cli_context_manager.set_silent),
is_flag=True,
rich_help_panel=_CLI_BEHAVIOUR,
is_eager=True,
)

VerboseOption = typer.Option(
Expand Down
8 changes: 6 additions & 2 deletions src/snowflake/cli/api/commands/snow_typer.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
from snowflake.cli.api.commands.flags import DEFAULT_CONTEXT_SETTINGS
from snowflake.cli.api.exceptions import CommandReturnTypeError
from snowflake.cli.api.output.types import CommandResult
from snowflake.cli.app.printing import print_result
from snowflake.cli.app.telemetry import flush_telemetry, log_command_usage

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -73,12 +71,16 @@ def pre_execute():
Pay attention to make this method safe to use if performed operations are not necessary
for executing the command in proper way.
"""
from snowflake.cli.app.telemetry import log_command_usage

log.debug("Executing command pre execution callback")
log_command_usage()

@staticmethod
def process_result(result):
"""Command result processor"""
from snowflake.cli.app.printing import print_result

# Because we still have commands like "logs" that do not return anything.
# We should improve it in future.
if not result:
Expand All @@ -100,5 +102,7 @@ def post_execute():
Callback executed after running any command callable. Pay attention to make this method safe to
use if performed operations are not necessary for executing the command in proper way.
"""
from snowflake.cli.app.telemetry import flush_telemetry

log.debug("Executing command post execution callback")
flush_telemetry()
2 changes: 1 addition & 1 deletion src/snowflake/cli/api/secure_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def temporary_directory(cls):
Works similarly to tempfile.TemporaryDirectory
"""
with tempfile.TemporaryDirectory(prefix="snowcli") as tmpdir:
with tempfile.TemporaryDirectory(prefix="snowflake-cli") as tmpdir:
log.info("Created temporary directory %s", tmpdir)
yield SecurePath(tmpdir)
log.info("Removing temporary directory %s", tmpdir)
Expand Down
4 changes: 2 additions & 2 deletions src/snowflake/cli/app/cli_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
setup_pycharm_remote_debugger_if_provided,
)
from snowflake.cli.app.main_typer import SnowCliMainTyper
from snowflake.cli.app.printing import print_result
from snowflake.cli.app.printing import MessageResult, print_result
from snowflake.connector.config_manager import CONFIG_MANAGER

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -103,7 +103,7 @@ def _commands_structure_callback(value: bool):
@_do_not_execute_on_completion
def _version_callback(value: bool):
if value:
typer.echo(f"Snowflake CLI version: {__about__.VERSION}")
print_result(MessageResult(f"Snowflake CLI version: {__about__.VERSION}"))
_exit_with_cleanup()


Expand Down
2 changes: 1 addition & 1 deletion src/snowflake/cli/app/loggers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from snowflake.cli.api.exceptions import InvalidLogsConfiguration
from snowflake.cli.api.secure_path import SecurePath

_DEFAULT_LOG_FILENAME = "snowcli.log"
_DEFAULT_LOG_FILENAME = "snowflake-cli.log"


@dataclass
Expand Down
4 changes: 2 additions & 2 deletions src/snowflake/cli/app/main_typer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
import sys

import typer
from rich import print as rich_print
from snowflake.cli.api.cli_global_context import cli_context
from snowflake.cli.api.commands.flags import DEFAULT_CONTEXT_SETTINGS, DebugOption
from snowflake.cli.api.console import cli_console


def _handle_exception(exception: Exception):
if cli_context.enable_tracebacks:
raise exception
else:
rich_print(
cli_console.warning(
"\nAn unexpected exception occurred. Use --debug option to see the traceback. Exception message:\n\n"
+ exception.__str__()
)
Expand Down
4 changes: 2 additions & 2 deletions src/snowflake/cli/plugins/connection/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import logging

import click
import typer
from click import ClickException, Context, Parameter # type: ignore
from click.core import ParameterSource # type: ignore
Expand All @@ -21,6 +20,7 @@
get_connection,
set_config_value,
)
from snowflake.cli.api.console import cli_console
from snowflake.cli.api.constants import ObjectType
from snowflake.cli.api.output.types import (
CollectionResult,
Expand Down Expand Up @@ -81,7 +81,7 @@ def callback(value: str):

def _password_callback(ctx: Context, param: Parameter, value: str):
if value and ctx.get_parameter_source(param.name) == ParameterSource.COMMANDLINE: # type: ignore
click.echo(PLAIN_PASSWORD_MSG)
cli_console.warning(PLAIN_PASSWORD_MSG)

return value

Expand Down
2 changes: 1 addition & 1 deletion src/snowflake/cli/plugins/snowpark/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def deploy(
stage_manager = StageManager()
stage_name = stage_manager.to_fully_qualified_name(stage_name)
stage_manager.create(
stage_name=stage_name, comment="deployments managed by snowcli"
stage_name=stage_name, comment="deployments managed by Snowflake CLI"
)

packages = get_snowflake_packages()
Expand Down
3 changes: 2 additions & 1 deletion src/snowflake/cli/plugins/spcs/image_repository/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from click import ClickException
from snowflake.cli.api.commands.flags import IfNotExistsOption, ReplaceOption
from snowflake.cli.api.commands.snow_typer import SnowTyper
from snowflake.cli.api.console import cli_console
from snowflake.cli.api.output.types import (
CollectionResult,
MessageResult,
Expand Down Expand Up @@ -126,7 +127,7 @@ def list_tags(
)

if response.status_code != 200:
print("Call to the registry failed", response.text)
cli_console.warning(f"Call to the registry failed {response.text}")

data = json.loads(response.text)
if "tags" in data:
Expand Down
4 changes: 2 additions & 2 deletions tests/__snapshots__/test_snow_connector.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use schema schemaValue;


create stage if not exists namedStageValue comment='deployments managed by snowcli';
create stage if not exists namedStageValue comment='deployments managed by Snowflake CLI';


put file://file_pathValue @namedStageValuepathValue auto_compress=false parallel=4 overwrite=overwriteValue;
Expand All @@ -45,7 +45,7 @@
use schema schemaValue;


create stage if not exists snow://embeddedStageValue comment='deployments managed by snowcli';
create stage if not exists snow://embeddedStageValue comment='deployments managed by Snowflake CLI';


put file://file_pathValue snow://embeddedStageValuepathValue auto_compress=false parallel=4 overwrite=overwriteValue;
Expand Down
6 changes: 3 additions & 3 deletions tests/api/commands/test_snow_typer.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,21 @@ def test_command_with_connection_options(cli, snapshot):
assert result.output == snapshot


@mock.patch("snowflake.cli.api.commands.snow_typer.log_command_usage")
@mock.patch("snowflake.cli.app.telemetry.log_command_usage")
def test_snow_typer_pre_execute_sends_telemetry(mock_log_command_usage, cli):
result = cli(app_factory(SnowTyper))(["simple_cmd", "Norma"])
assert result.exit_code == 0
mock_log_command_usage.assert_called_once_with()


@mock.patch("snowflake.cli.api.commands.snow_typer.flush_telemetry")
@mock.patch("snowflake.cli.app.telemetry.flush_telemetry")
def test_snow_typer_post_execute_sends_telemetry(mock_flush_telemetry, cli):
result = cli(app_factory(SnowTyper))(["simple_cmd", "Norma"])
assert result.exit_code == 0
mock_flush_telemetry.assert_called_once_with()


@mock.patch("snowflake.cli.api.commands.snow_typer.print_result")
@mock.patch("snowflake.cli.app.printing.print_result")
def test_snow_typer_result_callback_sends_telemetry(mock_print_result, cli):
result = cli(app_factory(SnowTyper))(["simple_cmd", "Norma"])
assert result.exit_code == 0
Expand Down
10 changes: 5 additions & 5 deletions tests/snowpark/test_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_deploy_function(

assert result.exit_code == 0, result.output
assert ctx.get_queries() == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project"
f" auto_compress=false parallel=4 overwrite=True",
dedent(
Expand Down Expand Up @@ -78,7 +78,7 @@ def test_deploy_function_with_external_access(

assert result.exit_code == 0, result.output
assert ctx.get_queries() == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project"
f" auto_compress=false parallel=4 overwrite=True",
dedent(
Expand Down Expand Up @@ -159,7 +159,7 @@ def test_deploy_function_no_changes(
}
]
assert queries == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project auto_compress=false parallel=4 overwrite=True",
]

Expand Down Expand Up @@ -197,7 +197,7 @@ def test_deploy_function_needs_update_because_packages_changes(
}
]
assert queries == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project auto_compress=false parallel=4 overwrite=True",
dedent(
"""\
Expand Down Expand Up @@ -246,7 +246,7 @@ def test_deploy_function_needs_update_because_handler_changes(
}
]
assert queries == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project"
f" auto_compress=false parallel=4 overwrite=True",
dedent(
Expand Down
4 changes: 2 additions & 2 deletions tests/snowpark/test_procedure.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_deploy_procedure(
]
)
assert ctx.get_queries() == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(tmp).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project auto_compress=false parallel=4 overwrite=True",
dedent(
"""\
Expand Down Expand Up @@ -117,7 +117,7 @@ def test_deploy_procedure_with_external_access(
]
)
assert ctx.get_queries() == [
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by snowcli'",
"create stage if not exists MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT comment='deployments managed by Snowflake CLI'",
f"put file://{Path(project_dir).resolve()}/app.zip @MOCKDATABASE.MOCKSCHEMA.DEV_DEPLOYMENT/my_snowpark_project"
f" auto_compress=false parallel=4 overwrite=True",
dedent(
Expand Down

0 comments on commit ac5bf54

Please sign in to comment.