Skip to content

Commit

Permalink
add backwards compatibility support
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-pczajka committed Mar 22, 2024
1 parent 76b267c commit 6587ca9
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 55 deletions.
26 changes: 10 additions & 16 deletions src/snowflake/cli/plugins/snowpark/package/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from typing import Optional

import typer
from click import ClickException
from snowflake.cli.api.commands.flags import (
deprecated_flag_callback,
deprecated_flag_callback_enum,
Expand Down Expand Up @@ -144,7 +143,7 @@ def package_upload(
help="Allows native libraries, when using packages installed through PIP",
hidden=True,
callback=deprecated_flag_callback_enum(
"--allow-native-libraries flag is no longer supported. Use --allow-shared-libraries flag instead."
"--allow-native-libraries flag is deprecated. Use --allow-shared-libraries flag instead."
),
)

Expand Down Expand Up @@ -172,7 +171,7 @@ def package_upload(
allow_shared_libraries_option = typer.Option(
False,
"--allow-shared-libraries",
help="Allows shared (.so) libraries, when using packages installed through PIP",
help="Allows shared (.so) libraries, when using packages installed through PIP.",
)


Expand All @@ -187,21 +186,20 @@ def package_create(
index_url: Optional[str] = index_option,
skip_version_check: bool = skip_version_check_option,
allow_shared_libraries: bool = allow_shared_libraries_option,
_deprecated_allow_native_libraries: PypiOption = deprecated_allow_native_libraries_option,
deprecated_allow_native_libraries: PypiOption = deprecated_allow_native_libraries_option,
_deprecated_install_option: bool = deprecated_install_option,
_deprecated_install_packages: bool = deprecated_pypi_download_option,
**options,
) -> CommandResult:
"""
Creates a Python package as a zip file that can be uploaded to a stage and imported for a Snowpark Python app.
"""
if _deprecated_allow_native_libraries == PypiOption.YES:
allow_shared_libraries = True
if _deprecated_allow_native_libraries == PypiOption.ASK:
raise ClickException(
"'ask' option of --allow-native-libraries is no longer supported."
" Use --allow-shared-libraries flag instead."
)
allow_shared_libraries_pypi_option = {
True: PypiOption.YES,
False: PypiOption.NO,
}[allow_shared_libraries]
if deprecated_allow_native_libraries != PypiOption.NO:
allow_shared_libraries_pypi_option = deprecated_allow_native_libraries

if ignore_anaconda:
anaconda = None
Expand All @@ -221,11 +219,7 @@ def package_create(
package_name=name,
file_name=None,
index_url=index_url,
# TODO: convert to boolean while refactoring "snowpark build"
allow_shared_libraries={
True: PypiOption.YES,
False: PypiOption.NO,
}[allow_shared_libraries],
allow_shared_libraries=allow_shared_libraries_pypi_option,
skip_version_check=skip_version_check,
)

Expand Down
33 changes: 15 additions & 18 deletions tests/__snapshots__/test_help_messages.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -2215,24 +2215,21 @@
│ [required] │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ Options ────────────────────────────────────────────────────────────────────╮
│ --ignore-anaconda Does not lookup packages on │
│ Snowflake Anaconda channel. │
│ --index-url TEXT Base URL of the Python │
│ Package Index to use for │
│ package lookup. This should │
│ point to a repository │
│ compliant with PEP 503 (the │
│ simple repository API) or a │
│ local directory laid out in │
│ the same format. │
│ --skip-version-check Skip comparing versions of │
│ dependencies between │
│ requirements and Anaconda. │
│ --allow-shared-libraries [yes|no|ask] Allows native libraries, │
│ when using packages │
│ installed through PIP │
│ [default: no] │
│ --help -h Show this message and exit. │
│ --ignore-anaconda Does not lookup packages on │
│ Snowflake Anaconda channel. │
│ --index-url TEXT Base URL of the Python Package Index │
│ to use for package lookup. This │
│ should point to a repository │
│ compliant with PEP 503 (the simple │
│ repository API) or a local directory │
│ laid out in the same format. │
│ --skip-version-check Skip comparing versions of │
│ dependencies between requirements │
│ and Anaconda. │
│ --allow-shared-libraries Allows shared (.so) libraries, when │
│ using packages installed through │
│ PIP. │
│ --help -h Show this message and exit. │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ Connection configuration ───────────────────────────────────────────────────╮
│ --connection,--environment -c TEXT Name of the connection, as defined │
Expand Down
34 changes: 28 additions & 6 deletions tests/snowpark/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@ def test_lookup_install_flag_are_deprecated(self, _, flags, runner):
in result.output
)

@mock.patch(
"snowflake.cli.plugins.snowpark.package.commands.AnacondaChannel.from_snowflake"
)
def test_lookup_install_with_out_flags_does_not_warn(self, _, runner):
result = runner.invoke(["snowpark", "package", "lookup", "foo"])
assert (
"is deprecated. Lookup command no longer checks for package in PyPi"
not in result.output
)

@pytest.mark.parametrize(
"flags",
[
Expand All @@ -210,15 +220,27 @@ def test_create_install_flag_are_deprecated(self, _, flags, runner):
in result.output
)

@pytest.mark.parametrize(
"flags",
[
["--allow-native-libraries", "yes"],
["--allow-native-libraries", "no"],
["--allow-native-libraries", "ask"],
],
)
@mock.patch(
"snowflake.cli.plugins.snowpark.package.commands.AnacondaChannel.from_snowflake"
)
def test_create_deprecated_flags_throw_warning(self, _, flags, runner):
result = runner.invoke(["snowpark", "package", "create", "foo", *flags])
assert "is deprecated." in result.output

@mock.patch(
"snowflake.cli.plugins.snowpark.package.commands.AnacondaChannel.from_snowflake"
)
def test_lookup_install_with_out_flags_does_not_warn(self, _, runner):
result = runner.invoke(["snowpark", "package", "lookup", "foo"])
assert (
"is deprecated. Lookup command no longer checks for package in PyPi"
not in result.output
)
def test_create_with_out_flags_does_not_warn(self, _, runner):
result = runner.invoke(["snowpark", "package", "create", "foo"])
assert "is deprecated" not in result.output

@staticmethod
def mocked_anaconda_response(response: dict):
Expand Down
31 changes: 16 additions & 15 deletions tests_integration/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,30 +103,31 @@ def test_create_package_with_deps(
assert any(["shrubbery.py" in file for file in files])

@pytest.mark.integration
@pytest.mark.parametrize("ignore_anaconda", (True, False))
@pytest.mark.parametrize(
"flags",
[
["--allow-shared-libraries"],
["--allow-native-libraries", "yes"],
["--allow-shared-libraries", "--ignore-anaconda"],
],
)
def test_package_with_conda_dependencies(
self, directory_for_test, runner, ignore_anaconda
self, directory_for_test, runner, flags
): # TODO think how to make this test with packages controlled by us
# test case is: We have a non-conda package, that has a dependency present on conda
# but not in latest version - here the case is matplotlib.
command = [
"snowpark",
"package",
"create",
"july",
"--allow-shared-libraries",
"yes",
]
if ignore_anaconda:
command.append("--ignore-anaconda")
result = runner.invoke_with_connection_json(command)
result = runner.invoke_with_connection(
["snowpark", "package", "create", "july", *flags]
)

assert result.exit_code == 0
assert Path("july.zip").exists()
assert Path("july.zip").exists(), result.output

files = self._get_filenames_from_zip("july.zip")
assert any(["colormaps.py" in name for name in files])
assert any(["matplotlib" in name for name in files]) == ignore_anaconda
assert any(["matplotlib" in name for name in files]) == (
"--ignore-anaconda" in flags
)

@pytest.mark.integration
def test_package_create_skip_version_check(self, directory_for_test, runner):
Expand Down

0 comments on commit 6587ca9

Please sign in to comment.