gemini-code-assist[bot] commented on code in PR #35849:
URL: https://github.com/apache/beam/pull/35849#discussion_r2268470448
##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -1529,13 +1529,20 @@ def wrapper(*args, **kwargs):
def _join_url_or_filepath(base, path):
if not base:
return path
- base_scheme = urllib.parse.urlparse(base, '').scheme
- path_scheme = urllib.parse.urlparse(path, base_scheme).scheme
- if path_scheme != base_scheme:
+
+ if urllib.parse.urlparse(path).scheme:
+ # path is an absolute path with scheme (whether it is the same as base or
+ # not).
return path
- elif base_scheme and base_scheme in urllib.parse.uses_relative:
+
+ # path is a relative path or an absolute path without scheme (e.g. /a/b/c)
+ base_scheme = urllib.parse.urlparse(base, '').scheme
+ if base_scheme and base_scheme in urllib.parse.uses_relative:
return urllib.parse.urljoin(base, path)
else:
+ if FileSystems.join(base, "") == base:
+ # base ends with a filesystem separator
+ return FileSystems.join(base, path)
return FileSystems.join(FileSystems.split(base)[0], path)
Review Comment:
The logic to handle filesystem paths can be slightly refactored to improve
readability by removing the nested `if/else` structure. The current
implementation is correct, but unifying the logic into a single return path can
make the intent clearer. The idea is to first determine the correct base
directory, and then perform the join. A base path without a trailing separator
is treated as a file, and its parent directory is used as the base for joining.
```suggestion
base_dir = base
if FileSystems.join(base, "") != base:
base_dir = FileSystems.split(base)[0]
return FileSystems.join(base_dir, path)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
