Skip to content

Commit 6a2883c

Browse files
committed
Fix local Windows path computation errors
- simplify URI analysis - fix URI usage in the runner
1 parent 2e9eab8 commit 6a2883c

2 files changed

Lines changed: 40 additions & 46 deletions

File tree

khiops/core/internals/filesystems.py

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import json
1010
import os
11-
import platform
1211
import shutil
1312
import warnings
1413
from abc import ABC, abstractmethod
@@ -59,8 +58,11 @@ def is_local_resource(uri_or_path):
5958
`bool`
6059
`True` if a URI refers to a local path
6160
"""
62-
uri_info = urlparse(uri_or_path, allow_fragments=False)
63-
return len(uri_info.scheme) <= 1 or uri_info.scheme == "file"
61+
if (index := uri_or_path.find("://")) > 0:
62+
scheme = uri_or_path[:index]
63+
return len(scheme) == 1 or scheme == "file"
64+
else:
65+
return True
6466

6567

6668
def create_resource(uri_or_path):
@@ -80,15 +82,34 @@ def create_resource(uri_or_path):
8082
`FilesystemResource`
8183
The URI resource object, its class depends on the URI.
8284
"""
83-
uri_info = urlparse(uri_or_path, allow_fragments=False)
84-
if uri_info.scheme == "s3":
85-
return AmazonS3Resource(uri_or_path)
86-
elif uri_info.scheme == "gs":
87-
return GoogleCloudStorageResource(uri_or_path)
88-
elif is_local_resource(uri_or_path):
89-
return LocalFilesystemResource(uri_or_path)
85+
# Case where the URI scheme separator `://` is contained in the uri/path
86+
if (index := uri_or_path.find("://")) > 0:
87+
scheme = uri_or_path[:index]
88+
89+
# Case of normal schemes (those whose scheme is not a single char)
90+
# Note: Any 1-char scheme is considered a Windows path
91+
if len(scheme) > 1:
92+
uri_info = urlparse(uri_or_path, allow_fragments=False)
93+
if uri_info.scheme == "s3":
94+
return AmazonS3Resource(uri_or_path)
95+
elif uri_info.scheme == "gs":
96+
return GoogleCloudStorageResource(uri_or_path)
97+
elif scheme == "file":
98+
# Reject URI if authority is not empty
99+
if uri_info.netloc:
100+
raise ValueError(
101+
f"Non-empty 'authority' in local-path URI '{uri_or_path}': "
102+
f"'{uri_info.netloc}'"
103+
)
104+
return LocalFilesystemResource(uri_or_path)
105+
else:
106+
raise ValueError(f"Unsupported URI scheme {uri_info.scheme}")
107+
else:
108+
return LocalFilesystemResource(uri_or_path)
109+
110+
# No scheme separator `://` found: Build a local resource
90111
else:
91-
raise ValueError(f"Unsupported URI scheme {uri_info.scheme}")
112+
return LocalFilesystemResource(uri_or_path)
92113

93114

94115
def parent_path(path):
@@ -411,12 +432,8 @@ def __init__(self, uri):
411432
# Obtain the local from the URI
412433
# Case where the scheme is in fact a windows drive
413434
# => Build the proper path with drive
414-
if (
415-
len(self.uri_info.scheme) == 1
416-
and self.uri_info.scheme.isalpha()
417-
and platform.system() == "Windows"
418-
):
419-
self.path = os.path.join(f"{self.uri_info.scheme}:\\", self.uri_info.path)
435+
if len(self.uri_info.scheme) == 1 and self.uri_info.scheme.isalpha():
436+
self.path = f"{self.uri_info.scheme}:{self.uri_info.path}"
420437
# Case of the "file" scheme
421438
elif self.uri_info.scheme == "file":
422439
# If invalid second colon in path (eg. "/C:/Users"):

khiops/core/internals/runner.py

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ def _get_dir_status(a_dir):
5959
# Remove initial slash on windows systems
6060
# urllib's url2pathname does not work properly
6161
a_dir_res = fs.create_resource(os.path.normpath(a_dir))
62-
a_dir_path = a_dir_res.uri_info.path
62+
63+
# a_dir_res is a LocalFilesystemResource already
64+
a_dir_path = a_dir_res.path
6365
if platform.system() == "Windows":
6466
if a_dir_path.startswith("/"):
6567
a_dir_path = a_dir_path[1:]
@@ -98,31 +100,6 @@ def _check_samples_dir(samples_dir):
98100
)
99101

100102

101-
def _extract_path_from_uri(uri):
102-
res = fs.create_resource(uri)
103-
if platform.system() == "Windows":
104-
# Case of file:///<LETTER>:/<REST_OF_PATH>:
105-
# Eliminate first slash ("/") from path if the first component
106-
if (
107-
res.uri_info.scheme == ""
108-
and res.uri_info.path[0] == "/"
109-
and res.uri_info.path[1].isalpha()
110-
and res.uri_info.path[2] == ":"
111-
):
112-
path = res.uri_info.path[1:]
113-
# Case of C:/<REST_OF_PATH>:
114-
# Just use the original path
115-
elif len(res.uri_info.scheme) == 1:
116-
path = uri
117-
# Otherwise return URI path as-is
118-
else:
119-
path = res.uri_info.path
120-
121-
else:
122-
path = res.uri_info.path
123-
return path
124-
125-
126103
def _khiops_env_file_exists(env_dir):
127104
"""Check ``khiops_env`` exists relative to the specified environment dir"""
128105
khiops_env_path = os.path.join(env_dir, "khiops_env")
@@ -399,7 +376,7 @@ def root_temp_dir(self):
399376
def root_temp_dir(self, dir_path):
400377
# Check existence, directory status and permissions for local paths
401378
if fs.is_local_resource(dir_path):
402-
real_dir_path = _extract_path_from_uri(dir_path)
379+
real_dir_path = fs.create_resource(dir_path).path
403380
if os.path.exists(real_dir_path):
404381
if os.path.isfile(real_dir_path):
405382
raise KhiopsEnvironmentError(
@@ -439,7 +416,7 @@ def create_temp_file(self, prefix, suffix):
439416
# Local resource: Effectively create the file with the python file API
440417
if fs.is_local_resource(self.root_temp_dir):
441418
# Extract the path from the potential URI
442-
root_temp_dir_path = _extract_path_from_uri(self.root_temp_dir)
419+
root_temp_dir_path = fs.create_resource(self.root_temp_dir).path
443420

444421
# Create the temporary file
445422
tmp_file_fd, tmp_file_path = tempfile.mkstemp(
@@ -470,7 +447,7 @@ def create_temp_dir(self, prefix):
470447
"""
471448
# Local resource: Effectively create the directory with the python file API
472449
if fs.is_local_resource(self.root_temp_dir):
473-
root_temp_dir_path = _extract_path_from_uri(self.root_temp_dir)
450+
root_temp_dir_path = fs.create_resource(self.root_temp_dir).path
474451
temp_dir = tempfile.mkdtemp(prefix=prefix, dir=root_temp_dir_path)
475452
# Remote resource: Just return a highly probable unique path
476453
else:

0 commit comments

Comments
 (0)