Skip to content

Commit 6ca501e

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

File tree

2 files changed

+33
-46
lines changed

2 files changed

+33
-46
lines changed

khiops/core/internals/filesystems.py

Lines changed: 27 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,8 @@ 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+
# Check if Path is local Windows path
62+
return isinstance(create_resource(uri_or_path), LocalFilesystemResource)
6463

6564

6665
def create_resource(uri_or_path):
@@ -80,15 +79,30 @@ def create_resource(uri_or_path):
8079
`FilesystemResource`
8180
The URI resource object, its class depends on the URI.
8281
"""
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)
82+
if (index := uri_or_path.find("://")) > 0:
83+
scheme = uri_or_path[:index]
84+
if len(scheme) > 1: # any 1-char scheme is considered a windows path
85+
uri_info = urlparse(uri_or_path, allow_fragments=False)
86+
if uri_info.scheme == "s3":
87+
return AmazonS3Resource(uri_or_path)
88+
elif uri_info.scheme == "gs":
89+
return GoogleCloudStorageResource(uri_or_path)
90+
else:
91+
raise ValueError(f"Unsupported URI scheme {uri_info.scheme}")
92+
elif scheme == "file":
93+
uri_info = urlparse(uri_or_path, allow_fragments=False)
94+
95+
# Reject URI if authority is not empty
96+
if uri_info.netloc:
97+
raise ValueError(
98+
f"Non-empty 'autority' in local-path URI '{uri_or_path}': "
99+
f"'{uri_info.netloc}'"
100+
)
101+
return LocalFilesystemResource(uri_or_path)
102+
else:
103+
return LocalFilesystemResource(uri_or_path)
90104
else:
91-
raise ValueError(f"Unsupported URI scheme {uri_info.scheme}")
105+
return LocalFilesystemResource(uri_or_path)
92106

93107

94108
def parent_path(path):
@@ -411,12 +425,8 @@ def __init__(self, uri):
411425
# Obtain the local from the URI
412426
# Case where the scheme is in fact a windows drive
413427
# => 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)
428+
if len(self.uri_info.scheme) == 1 and self.uri_info.scheme.isalpha():
429+
self.path = f"{self.uri_info.scheme}:{self.uri_info.path}"
420430
# Case of the "file" scheme
421431
elif self.uri_info.scheme == "file":
422432
# 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)