[python] use str to replace pathlib.Path to avoid scheme missing in pypaimon #6604
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
XiaoHongbo-Hope
changed the title
Contributor
|
Maybe we can just use str. |
XiaoHongbo-Hope
force-pushed
the
read_blob-as-descriptor_dev
branch
from
5b4721f to
e87ada1
Compare
XiaoHongbo-Hope
force-pushed
the
read_blob-as-descriptor_dev
branch
from
e87ada1 to
e4d22d6
Compare
XiaoHongbo-Hope
changed the title
lucasfang
reviewed
Nov 19, 2025
| return self._trim_schema(self.warehouse) / f"{name}{Catalog.DB_SUFFIX}" | ||
| def get_database_path(self, name) -> str: | ||
| warehouse = self.warehouse.rstrip('/') | ||
| return f"{warehouse}/{name}{Catalog.DB_SUFFIX}" |
Contributor
There was a problem hiding this comment.
how about use os.path.join to construct the path? then you don’t need to use rstrip to remove trailing slashes from the warehouse path
Contributor
Author
There was a problem hiding this comment.
how about use os.path.join to construct the path? then you don’t need to use rstrip to remove trailing slashes from the warehouse path
Thanks for your kind suggestion. I tried use os.path but found that
os.path.join(self.warehouse, "db.db")producesoss://bucket/path\db.dbon Windows instead ofoss://bucket/path/db.db. So I choose str concat finally
Contributor
|
+1 |
2 similar comments
Contributor
|
+1 |
Contributor
|
+1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Fixes URI scheme loss issue in blob-as-descriptor mode by replacing
pathlib.Pathwithstrthroughout the codebase. This ensures URI schemes (e.g.,oss://,s3://,file://) are preserved when handling blob descriptors and file paths.Key Changes
Replaced
pathlib.Pathwithstr: Unified path handling to preserve URI schemes across all filesystem operations.Impact
FileIO,FileSystemCatalog,RESTCatalog- all path operations usestrDataFileMeta,SchemaManager, manifest managers - - path storage usestrFormatBlobReader,DataWriter,BlobWriter, etc.) - consistentstrpath handlingFileStoreTable,SnapshotManager--table path usestrBlob,BlobDescriptor- URI scheme preservation for external blob storageTests
test_blob_write_read_end_to_end_with_descriptor: Verifies URI scheme preservation in blob descriptorsoss_blob_as_descriptor.py: Demonstrates OSS blob-as-descriptor functionality with scheme preservationtest_file_iorest_token_file_io