Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-pczajka committed Aug 26, 2024
1 parent 8250288 commit 91829c9
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 12 deletions.
5 changes: 2 additions & 3 deletions src/snowflake/cli/_plugins/nativeapp/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
from snowflake.cli._plugins.stage.manager import StageManager
from snowflake.cli._plugins.stage.utils import print_diff_to_console
from snowflake.cli.api.console import cli_console as cc
from snowflake.cli.api.constants import DEFAULT_SIZE_LIMIT_MB
from snowflake.cli.api.errno import (
DOES_NOT_EXIST_OR_CANNOT_BE_PERFORMED,
DOES_NOT_EXIST_OR_NOT_AUTHORIZED,
Expand All @@ -90,7 +89,7 @@
from snowflake.cli.api.rendering.sql_templates import (
snowflake_sql_jinja_render,
)
from snowflake.cli.api.secure_path import SecurePath
from snowflake.cli.api.secure_path import UNLIMITED, SecurePath
from snowflake.cli.api.sql_execution import SqlExecutionMixin
from snowflake.connector import DictCursor, ProgrammingError

Expand Down Expand Up @@ -600,7 +599,7 @@ def _render_script_templates(
script_full_path = SecurePath(self.project_root) / relpath
try:
template_content = script_full_path.read_text(
file_size_limit_mb=DEFAULT_SIZE_LIMIT_MB
file_size_limit_mb=UNLIMITED
)
result = render_from_str(template_content, jinja_context)
scripts_contents.append(result)
Expand Down
9 changes: 1 addition & 8 deletions src/snowflake/cli/api/rendering/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,6 @@ def jinja_render_from_str(template_content: str, data: Dict[str, Any]) -> str:
return _get_jinja_env().from_string(template_content).render(data)


def path_to_jinja_pathlike_str(path: Path) -> str:
# jinja2 template loader user '/' as path separator (even on Windows)
return "/".join(path.parts)


def jinja_render_from_file(
template_path: Path, data: Dict[str, Any], output_file_path: Optional[Path] = None
) -> Optional[str]:
Expand All @@ -126,9 +121,7 @@ def jinja_render_from_file(
None if file path is provided, else returns the rendered string.
"""
env = _get_jinja_env(
loader=loaders.FileSystemLoader(
path_to_jinja_pathlike_str(template_path.parent)
)
loader=loaders.FileSystemLoader(template_path.parent.as_posix())
)
loaded_template = env.get_template(template_path.name)
rendered_result = loaded_template.render(**data)
Expand Down
5 changes: 5 additions & 0 deletions src/snowflake/cli/api/rendering/sql_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from click import ClickException
from jinja2 import Environment, StrictUndefined, loaders, meta
from snowflake.cli.api.cli_global_context import get_cli_context
from snowflake.cli.api.console.console import cli_console
from snowflake.cli.api.exceptions import InvalidTemplate
from snowflake.cli.api.rendering.jinja import (
CONTEXT_KEY,
Expand Down Expand Up @@ -65,6 +66,10 @@ def choose_sql_jinja_env_based_on_template_syntax(template_content: str) -> Envi
f" and {_SQL_TEMPLATE_START} ... {_SQL_TEMPLATE_END} syntax."
)
if has_old_syntax:
cli_console.warning(
f"Warning: {_OLD_SQL_TEMPLATE_START} ... {_OLD_SQL_TEMPLATE_END} syntax is deprecated."
f" Use {_SQL_TEMPLATE_START} ... {_SQL_TEMPLATE_END} syntax instead."
)
return old_syntax_env
return new_syntax_env

Expand Down
12 changes: 11 additions & 1 deletion tests/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# 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.

from pathlib import Path
from tempfile import NamedTemporaryFile, TemporaryDirectory
from unittest import mock
Expand Down Expand Up @@ -333,6 +332,17 @@ def test_rendering_of_sql(mock_execute_query, query, runner):
)


@mock.patch("snowflake.cli._plugins.sql.commands.SqlManager._execute_string")
def test_old_template_syntax_causes_warning(mock_execute_query, runner):
result = runner.invoke(["sql", "-q", "select &{ aaa }", "-D", "aaa=foo"])
assert result.exit_code == 0
assert (
"Warning: &{ ... } syntax is deprecated. Use <% ... %> syntax instead."
in result.output
)
mock_execute_query.assert_called_once_with("select foo", cursor_class=VerboseCursor)


@mock.patch("snowflake.cli._plugins.sql.commands.SqlManager._execute_string")
def test_mixed_template_syntax_error(mock_execute_query, runner):
result = runner.invoke(
Expand Down

0 comments on commit 91829c9

Please sign in to comment.