Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for package creation and pip installs #781

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
9466af8
Added dependencies check to venv
sfc-gh-jsikorski Feb 14, 2024
8478785
Problem
sfc-gh-jsikorski Feb 14, 2024
96de24c
Problem2
sfc-gh-jsikorski Feb 14, 2024
e1968e9
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 14, 2024
2d7d2a0
Fixed problem
sfc-gh-jsikorski Feb 15, 2024
0c0fecf
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 15, 2024
d628f82
Cleanup
sfc-gh-jsikorski Feb 15, 2024
5a5dcca
Corrected github installs
sfc-gh-jsikorski Feb 16, 2024
f9fe510
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 16, 2024
4db4a36
Corrected lib path
sfc-gh-jsikorski Feb 16, 2024
184ab17
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 16, 2024
ad58610
added release notes
sfc-gh-jsikorski Feb 19, 2024
ce0ab68
fixes
sfc-gh-jsikorski Feb 19, 2024
6de81a3
fixes
sfc-gh-jsikorski Feb 19, 2024
553a7fd
fixes
sfc-gh-jsikorski Feb 19, 2024
a78306a
fixes
sfc-gh-jsikorski Feb 19, 2024
63b81c5
Added native libraries option
sfc-gh-jsikorski Feb 19, 2024
0568766
Added native libraries option
sfc-gh-jsikorski Feb 19, 2024
8758573
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 19, 2024
303d686
Snapshot update
sfc-gh-jsikorski Feb 19, 2024
2cd4957
Snapshot update
sfc-gh-jsikorski Feb 19, 2024
9643779
Fixes
sfc-gh-jsikorski Feb 20, 2024
e364798
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 20, 2024
3f12e90
After merge fixes
sfc-gh-jsikorski Feb 20, 2024
d04a4af
Minor
sfc-gh-jsikorski Feb 20, 2024
e6e8734
fixes
sfc-gh-jsikorski Feb 21, 2024
b913bd8
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 21, 2024
ffee145
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 21, 2024
fac040e
Updates
sfc-gh-jsikorski Feb 21, 2024
6eacdb9
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 23, 2024
4473db3
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 27, 2024
5a30415
Performance update
sfc-gh-jsikorski Feb 28, 2024
e3f6196
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Feb 28, 2024
2911571
Test fix
sfc-gh-jsikorski Feb 28, 2024
5809275
windows fix
sfc-gh-jsikorski Feb 28, 2024
3cd0016
windows fix
sfc-gh-jsikorski Feb 28, 2024
4c6c737
windows fix
sfc-gh-jsikorski Feb 28, 2024
b9afc2a
windows fix
sfc-gh-jsikorski Feb 29, 2024
44ebaa1
windows fix
sfc-gh-jsikorski Feb 29, 2024
9dd57b7
windows fix
sfc-gh-jsikorski Feb 29, 2024
54221b0
windows fix
sfc-gh-jsikorski Feb 29, 2024
24f2135
windows fix
sfc-gh-jsikorski Mar 1, 2024
8a9d663
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 1, 2024
28ba4ee
fixed tests
sfc-gh-jsikorski Mar 1, 2024
abcd0fd
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 1, 2024
346f7eb
fixed tests
sfc-gh-jsikorski Mar 1, 2024
420f037
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 7, 2024
dd05ba7
Fixes after review
sfc-gh-jsikorski Mar 7, 2024
024435e
Fixes after review
sfc-gh-jsikorski Mar 8, 2024
7ba5714
Fixes
sfc-gh-jsikorski Mar 8, 2024
50efec9
Fixes
sfc-gh-jsikorski Mar 8, 2024
27d1d0c
Fixes
sfc-gh-jsikorski Mar 8, 2024
8df3710
Fixes
sfc-gh-jsikorski Mar 8, 2024
852a4dd
Fixes
sfc-gh-jsikorski Mar 8, 2024
01a0ae0
Fixes
sfc-gh-jsikorski Mar 8, 2024
21521b7
Fixes
sfc-gh-jsikorski Mar 8, 2024
2578038
Fixes
sfc-gh-jsikorski Mar 11, 2024
cc8023a
Replaced deprecated flags
sfc-gh-jsikorski Mar 11, 2024
27aba75
Replaced deprecated flags
sfc-gh-jsikorski Mar 11, 2024
10bfcc9
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 11, 2024
d9b999d
Replaced deprecated flags
sfc-gh-jsikorski Mar 11, 2024
1654c57
Replaced deprecated flags
sfc-gh-jsikorski Mar 11, 2024
6689b91
Fix for encoding
sfc-gh-jsikorski Mar 11, 2024
f57aad5
Fix for encoding
sfc-gh-jsikorski Mar 11, 2024
cd7ba59
Fix for encoding
sfc-gh-jsikorski Mar 11, 2024
4f610c8
Fix for encoding
sfc-gh-jsikorski Mar 11, 2024
654d54f
Fix for encoding
sfc-gh-jsikorski Mar 11, 2024
f767a18
Started cleaning
sfc-gh-jsikorski Mar 11, 2024
d95501f
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 11, 2024
2277f88
Cleanup
sfc-gh-jsikorski Mar 11, 2024
9ba815c
Merge branch 'main' into jsikorski/SNOW-1053803-fix-anaconda-check-in…
sfc-gh-jsikorski Mar 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ repos:
^src/snowflake/cli/app/dev/.*$|
^src/snowflake/cli/templates/.*$|
^src/snowflake/cli/api/utils/rendering.py$|
^src/snowflake/cli/plugins/spcs/common.py$
^src/snowflake/cli/plugins/spcs/common.py$|
^src/snowflake/cli/plugins/snowpark/venv.py$
- id: check-app-imports-in-api
language: pygrep
name: "No top level cli.app imports in cli.api"
Expand Down
4 changes: 4 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
* Added `create` command for `image-repository`.
* Added `status`, `set (property)`, `unset (property)`, `suspend` and `resume` commands for `compute-pool`.
* Added `set (property)`, `unset (property)`,`upgrade` and `list-endpoints` commands for `service`.
* You can now use github repo link in `snow snowpark package create` to prepare your code for upload
* Added `allow-native-libraries` option to `snow snowpark package create` command
* Added alias `--install-from-pip` for `-y` option in `snow snowpark package create` command
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make -y deprecated, I think this will be a nice exercise in how to do this (two different flags? callback?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently they are two different flags - fixed in #811

* Connections parameters are also supported by generic environment variables:
* `SNOWFLAKE_ACCOUNT`
* `SNOWFLAKE_USER`
Expand All @@ -49,6 +52,7 @@
## Fixes and improvements
* Restricted permissions of automatically created files
* Fixed bug where `spcs service create` would not throw error if service with specified name already exists.
* Improved package lookup, to avoid unnecessary uploads
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add more info?

* Logging into the file by default (INFO level)
* Added validation that service, compute pool, and image repository names are unqualified identifiers.
* `spcs service` commands now accept qualified names.
Expand Down
67 changes: 66 additions & 1 deletion src/snowflake/cli/plugins/snowpark/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from __future__ import annotations

import re
from dataclasses import dataclass
from enum import Enum
from typing import List

from requirements.requirement import Requirement
from requirements import requirement


class PypiOption(Enum):
Expand All @@ -13,6 +14,35 @@ class PypiOption(Enum):
ASK = "ask"


class RequirementType(Enum):
FILE = "file"
PACKAGE = "package"


class Requirement(requirement.Requirement):
extra_pattern = re.compile("'([^']*)'")

@classmethod
def parse_line(cls, line: str) -> Requirement:
if len(line_elements := line.split(";")) > 1:
line = line_elements[0]
result = super().parse_line(line)

if len(line_elements) > 1:
for element in line_elements[1:]:
if "extra" in element and (extras := cls.extra_pattern.search(element)):
result.extras.extend(extras.groups())

if result.uri and not result.name:
result.name = get_package_name(result.uri)

return result

@classmethod
def _look_for_specifier(cls, specifier: str, line: str):
return re.search(cls.specifier_pattern.format(specifier), line)


@dataclass
class SplitRequirements:
"""A dataclass to hold the results of parsing requirements files and dividing them into
Expand All @@ -30,3 +60,38 @@ class RequirementWithFiles:

requirement: Requirement
files: List[str]


@dataclass
class RequirementWithFilesAndDeps(RequirementWithFiles):
dependencies: List[str]

def to_requirement_with_files(self):
return RequirementWithFiles(self.requirement, self.files)


def get_package_name(name: str) -> str:
if name.lower().startswith(("git+", "http")):
pattern = re.compile(r"github\.com\/[^\/]+\/([^\/][^.@$/]+)")
if match := pattern.search(name):
return match.group(1)
else:
return name

elif name.endswith(".zip"):
return name.replace(".zip", "")
else:
return name


pip_failed_msg = (
"pip failed with return code {}."
"If pip is installed correctly, this may mean you`re trying to install a package"
"that isn't compatible with the host architecture -"
"and generally means it has native libraries."
)
second_chance_msg = (
"Good news! The following package dependencies can be"
"imported directly from Anaconda, and will be excluded from"
"the zip: {}"
)
25 changes: 17 additions & 8 deletions src/snowflake/cli/plugins/snowpark/package/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import typer
from snowflake.cli.api.commands.snow_typer import SnowTyper
from snowflake.cli.api.output.types import CommandResult, MessageResult
from snowflake.cli.plugins.snowpark.models import PypiOption
from snowflake.cli.plugins.snowpark.package.manager import (
cleanup_after_install,
create,
Expand All @@ -17,6 +18,7 @@
NotInAnaconda,
RequiresPackages,
)
from snowflake.cli.plugins.snowpark.snowpark_shared import PackageNativeLibrariesOption

app = SnowTyper(
name="package",
Expand Down Expand Up @@ -45,6 +47,7 @@ def package_lookup(
name: str = typer.Argument(..., help="Name of the package."),
install_packages: bool = install_option,
deprecated_install_option: bool = deprecated_install_option,
allow_native_libraries: PypiOption = PackageNativeLibrariesOption,
**options,
) -> CommandResult:
"""
Expand All @@ -55,7 +58,11 @@ def package_lookup(
if deprecated_install_option:
install_packages = deprecated_install_option

lookup_result = lookup(name=name, install_packages=install_packages)
lookup_result = lookup(
name=name,
install_packages=install_packages,
allow_native_libraries=allow_native_libraries,
)
return MessageResult(lookup_result.message)


Expand Down Expand Up @@ -97,6 +104,7 @@ def package_create(
),
install_packages: bool = install_option,
deprecated_install_option: bool = deprecated_install_option,
allow_native_libraries: PypiOption = PackageNativeLibrariesOption,
**options,
) -> CommandResult:
"""
Expand All @@ -105,13 +113,14 @@ def package_create(
if deprecated_install_option:
install_packages = deprecated_install_option

if (
type(lookup_result := lookup(name=name, install_packages=install_packages))
in [
NotInAnaconda,
RequiresPackages,
]
and type(creation_result := create(name)) == CreatedSuccessfully
lookup_result = lookup(
name=name,
install_packages=install_packages,
allow_native_libraries=allow_native_libraries,
)

if isinstance(lookup_result, (NotInAnaconda, RequiresPackages)) and isinstance(
creation_result := create(name), CreatedSuccessfully
):
message = creation_result.message
if type(lookup_result) == RequiresPackages:
Expand Down
19 changes: 14 additions & 5 deletions src/snowflake/cli/plugins/snowpark/package/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@
from functools import wraps
from pathlib import Path

from requirements.requirement import Requirement
from snowflake.cli.api.constants import PACKAGES_DIR
from snowflake.cli.api.secure_path import SecurePath
from snowflake.cli.plugins.object.stage.manager import StageManager
from snowflake.cli.plugins.snowpark import package_utils
from snowflake.cli.plugins.snowpark.models import SplitRequirements
from snowflake.cli.plugins.snowpark.models import (
PypiOption,
Requirement,
SplitRequirements,
get_package_name,
)
from snowflake.cli.plugins.snowpark.package.utils import (
CreatedSuccessfully,
InAnaconda,
Expand All @@ -25,15 +29,20 @@
log = logging.getLogger(__name__)


def lookup(name: str, install_packages: bool) -> LookupResult:
def lookup(
name: str, install_packages: bool, allow_native_libraries: PypiOption
) -> LookupResult:

package_response = package_utils.parse_anaconda_packages([Requirement.parse(name)])

if package_response.snowflake and not package_response.other:
return InAnaconda(package_response, name)
elif install_packages:
status, result = package_utils.install_packages(
perform_anaconda_check=True, package_name=name, file_name=None
perform_anaconda_check=True,
package_name=name,
file_name=None,
allow_native_libraries=allow_native_libraries,
)

if status:
Expand Down Expand Up @@ -65,7 +74,7 @@ def upload(file: Path, stage: str, overwrite: bool):


def create(zip_name: str):
file_name = zip_name if zip_name.endswith(".zip") else f"{zip_name}.zip"
file_name = f"{get_package_name(zip_name)}.zip"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we moved the .zip suffix to get_package_name function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_package_name is used to extract name, that we will use to resolve dependencies etc. - so for example, takes the name from github repo url, zipname etc. - it seems way too early to add .zip suffix there

zip_dir(dest_zip=Path(file_name), source=Path.cwd() / ".packages")

if os.path.exists(file_name):
Expand Down
3 changes: 1 addition & 2 deletions src/snowflake/cli/plugins/snowpark/package/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
from pathlib import Path
from typing import List

from requirements.requirement import Requirement
from snowflake.cli.api.secure_path import SecurePath
from snowflake.cli.plugins.snowpark.models import SplitRequirements
from snowflake.cli.plugins.snowpark.models import Requirement, SplitRequirements


@dataclass
Expand Down
Loading
Loading