This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3604-4fb9e68c7931538db019d2d92a77aa7a32e3c716 in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit 131b18712f11d695ad4cd47b682f37750dcf3ba6 Author: Anas Khan <[email protected]> AuthorDate: Sat Oct 24 06:40:59 2026 +0000 fix: build StaticTable version-hint metadata locations with forward slashes (#3604) * fix: build StaticTable version-hint metadata locations with forward slashes StaticTable._metadata_location_from_version_hint built the version-hint and resolved metadata locations with os.path.join. Iceberg locations are URIs and must always be forward-slash separated, but os.path.join uses the OS separator, so on Windows it produced backslash-separated keys such as s3://.../taxis\metadata\version-hint.text. That is an invalid object-store key, which breaks the documented "pass the table root path" usage of StaticTable.from_metadata on Windows. Join the components with explicit forward slashes, matching how locations are built elsewhere in pyiceberg (for example pyiceberg/table/locations.py). Behavior is unchanged on POSIX. The now-unused os import is removed. Add an OS-independent regression test that swaps os.path.join for the Windows join so the metadata location is asserted to stay a valid forward-slash URI on any platform (the existing test only exercised local temp paths, which use the POSIX separator on Linux CI and never caught this). Related: #2477 * Test version-hint resolution with real files instead of fakes Replace the fake FileIO test with one that writes metadata and version-hint files under a file:// URI and asserts the resolved metadata location. Windows CI catches a regression to os.path.join. Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: Kevin Liu <[email protected]> Co-authored-by: Copilot App <[email protected]> --- pyiceberg/table/__init__.py | 11 ++++++----- tests/table/test_init.py | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pyiceberg/table/__init__.py b/pyiceberg/table/__init__.py index 9c38da3b2..7da0e59d2 100644 --- a/pyiceberg/table/__init__.py +++ b/pyiceberg/table/__init__.py @@ -18,7 +18,6 @@ from __future__ import annotations import itertools import logging -import os import random import time import uuid @@ -1970,7 +1969,9 @@ class StaticTable(Table): @classmethod def _metadata_location_from_version_hint(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> str: - version_hint_location = os.path.join(metadata_location, "metadata", "version-hint.text") + # Using os.path.join here would produce "\" separators on Windows, breaking URI locations + metadata_dir = f"{metadata_location.rstrip('/')}/metadata" + version_hint_location = f"{metadata_dir}/version-hint.text" io = load_file_io(properties=properties, location=version_hint_location) file = io.new_input(version_hint_location) @@ -1978,11 +1979,11 @@ class StaticTable(Table): content = stream.read().decode("utf-8") if content.endswith(".metadata.json"): - return os.path.join(metadata_location, "metadata", content) + return f"{metadata_dir}/{content}" elif content.isnumeric(): - return os.path.join(metadata_location, "metadata", f"v{content}.metadata.json") + return f"{metadata_dir}/v{content}.metadata.json" else: - return os.path.join(metadata_location, "metadata", f"{content}.metadata.json") + return f"{metadata_dir}/{content}.metadata.json" @classmethod def from_metadata(cls, metadata_location: str, properties: Properties = EMPTY_DICT) -> StaticTable: diff --git a/tests/table/test_init.py b/tests/table/test_init.py index f4b63bc41..94614c9d6 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -18,6 +18,7 @@ import json import uuid from copy import copy +from pathlib import Path from typing import Any import pytest @@ -37,6 +38,7 @@ from pyiceberg.expressions.visitors import bind from pyiceberg.io import PY_IO_IMPL, FileIO, load_file_io from pyiceberg.partitioning import PartitionField, PartitionSpec from pyiceberg.schema import Schema +from pyiceberg.serializers import ToOutputFile from pyiceberg.table import ( CommitTableRequest, StagedTable, @@ -615,6 +617,26 @@ def test_static_table_version_hint_same_as_table( assert static_table.metadata == table_v2.metadata +def test_static_table_version_hint_uri_location(table_v2: Table, tmp_path: Path) -> None: + # Fails on Windows if the locations are built with os.path.join + table_root = f"file://{tmp_path}" + file_io = load_file_io(location=table_root) + + for version_hint_content, metadata_file in [ + ("v3.metadata.json", "v3.metadata.json"), + ("3", "v3.metadata.json"), + ("some-uuid", "some-uuid.metadata.json"), + ]: + metadata_location = f"{table_root}/metadata/{metadata_file}" + ToOutputFile.table_metadata(table_v2.metadata, file_io.new_output(metadata_location), overwrite=True) + with file_io.new_output(f"{table_root}/metadata/version-hint.text").create(overwrite=True) as f: + f.write(version_hint_content.encode("utf-8")) + + static_table = StaticTable.from_metadata(table_root) + assert static_table.metadata_location == metadata_location + assert static_table.metadata == table_v2.metadata + + def test_static_table_io_does_not_exist(metadata_location: str) -> None: with pytest.raises(ValueError): StaticTable.from_metadata(metadata_location, {PY_IO_IMPL: "pyiceberg.does.not.exist.FileIO"})
