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.

Sign up for GitHub

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

accept stage paths starting with / for GET #2889

Merged
merged 3 commits into from
Jan 28, 2025
Merged

accept stage paths starting with / for GET #2889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c8c009
accept stage paths starting with / for GET
sfc-gh-tvanderkooy Jan 20, 2025
96de4a1
Keep existing behaviour for `session.read.<file_format>(<path>)`, tha…
sfc-gh-tvanderkooy Jan 27, 2025
fa61064
Add changelog entry
sfc-gh-tvanderkooy Jan 28, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
- Added support for creating a temporary view via `DataFrame.create_or_replace_temp_view` from a DataFrame created by reading a file from a stage.
- Added support for `value_contains_null` parameter to MapType.
- Added `interactive` to telemetry that indicates whether the current environment is an interactive one.
- Allow `session.file.get` in a Native App to read file paths starting with `/` from the current version

#### Bug Fixes

Expand Down
8 changes: 6 additions & 2 deletions src/snowflake/snowpark/_internal/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,14 @@

STAGE_PREFIX = "@"
SNOWURL_PREFIX = "snow://"
RELATIVE_PATH_PREFIX = "/"
SNOWFLAKE_PATH_PREFIXES = [
STAGE_PREFIX,
SNOWURL_PREFIX,
]
SNOWFLAKE_PATH_PREFIXES_FOR_GET = SNOWFLAKE_PATH_PREFIXES + [
RELATIVE_PATH_PREFIX,
]

# Scala uses 3 but this can be larger. Consider allowing users to configure it.
QUERY_TAG_TRACEBACK_LIMIT = 3
Expand Down Expand Up @@ -372,7 +376,7 @@ def normalize_path(path: str, is_local: bool) -> str:
a directory named "load data". Therefore, if `path` is already wrapped by single quotes,
we do nothing.
"""
prefixes = ["file://"] if is_local else SNOWFLAKE_PATH_PREFIXES
prefixes = ["file://"] if is_local else SNOWFLAKE_PATH_PREFIXES_FOR_GET
Comment on lines -375 to +379
Copy link
Contributor

Choose a reason for hiding this comment

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

this change adds support when run inside a native apps environment. How does it affect when run outside NA? If it is not supported, then we should only allow the new prefix in NA env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Outside native apps this will be a SQL error. SQL compilation error: invalid URL prefix found in: '/some/path'

But that's the current behaviour too (for GET) - today if you do session.file.get('/some/path'), snowpark will prepend @ and run the GET, and then throw SQL compilation error: missing stage name in URL: @/some/path

Unfortunately I'm not aware of a good way to check if this is NA env, it might be difficult to check that here.

if is_single_quoted(path):
return path
if is_local and OPERATING_SYSTEM == "Windows":
Expand Down Expand Up @@ -408,7 +412,7 @@ def split_path(path: str) -> Tuple[str, str]:

def unwrap_stage_location_single_quote(name: str) -> str:
new_name = unwrap_single_quote(name)
if any(new_name.startswith(prefix) for prefix in SNOWFLAKE_PATH_PREFIXES):
if any(new_name.startswith(prefix) for prefix in SNOWFLAKE_PATH_PREFIXES_FOR_GET):
return new_name
return f"{STAGE_PREFIX}{new_name}"

Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_internal_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ def test_split_path(path: str, expected_dir: str, expected_file: str) -> None:
False,
"'snow://domain/test_entity/versions/test_version/file.txt'",
),
("/some/file.yml", False, "'/some/file.yml'"),
("'/some/file.yml'", False, "'/some/file.yml'"),
],
)
def test_normalize_path(path: str, is_local: bool, expected: str) -> None:
Expand Down
Loading