Skip to content

Commit

Permalink
fix windows path (#660) (#673)
Browse files Browse the repository at this point in the history
## Changes
<!-- Summary of your changes that are easy to understand -->
Changed `pathlib.Path` with the `pathlib.PurePosixPath` in
`/databricks/sdk/mixins/files.py` which always use linux path separators
regardless of the OS that it is running on. Fixes (#660)

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] `make fmt` applied
- [ ] relevant integration tests applied
  • Loading branch information
parthban-db committed Jul 3, 2024
1 parent 228cc8f commit f0ac1da
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
dev:
python3 -m venv .venv
ifeq ($(OS), Windows_NT)
.venv\Scripts\activate
else
. .venv/bin/activate
endif
pip install '.[dev]'

install:
Expand Down
16 changes: 12 additions & 4 deletions databricks/sdk/mixins/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import base64
import os
import pathlib
import platform
import shutil
import sys
from abc import ABC, abstractmethod
Expand Down Expand Up @@ -266,8 +267,9 @@ def __repr__(self) -> str:

class _Path(ABC):

def __init__(self, path: str):
self._path = pathlib.Path(str(path).replace('dbfs:', '').replace('file:', ''))
@abstractmethod
def __init__(self):
...

@property
def is_local(self) -> bool:
Expand Down Expand Up @@ -327,6 +329,12 @@ def as_string(self) -> str:

class _LocalPath(_Path):

def __init__(self, path: str):
if platform.system() == "Windows":
self._path = pathlib.Path(str(path).replace('file:///', '').replace('file:', ''))
else:
self._path = pathlib.Path(str(path).replace('file:', ''))

def _is_local(self) -> bool:
return True

Expand Down Expand Up @@ -393,7 +401,7 @@ def __repr__(self) -> str:
class _VolumesPath(_Path):

def __init__(self, api: files.FilesAPI, src: Union[str, pathlib.Path]):
super().__init__(src)
self._path = pathlib.PurePosixPath(str(src).replace('dbfs:', '').replace('file:', ''))
self._api = api

def _is_local(self) -> bool:
Expand Down Expand Up @@ -462,7 +470,7 @@ def __repr__(self) -> str:
class _DbfsPath(_Path):

def __init__(self, api: files.DbfsAPI, src: str):
super().__init__(src)
self._path = pathlib.PurePosixPath(str(src).replace('dbfs:', '').replace('file:', ''))
self._api = api

def _is_local(self) -> bool:
Expand Down
30 changes: 30 additions & 0 deletions tests/test_dbfs_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,33 @@ def test_fs_path_invalid(config):
with pytest.raises(ValueError) as e:
dbfs_ext._path('s3://path/to/file')
assert 'unsupported scheme "s3"' in str(e.value)


def test_dbfs_local_path_mkdir(config, tmp_path):
from databricks.sdk import WorkspaceClient

w = WorkspaceClient(config=config)
w.dbfs._path(f'file:{tmp_path}/test_dir').mkdir()
assert w.dbfs.exists(f'file:{tmp_path}/test_dir')


def test_dbfs_exists(config, mocker):
from databricks.sdk import WorkspaceClient

get_status = mocker.patch('databricks.sdk.service.files.DbfsAPI.get_status', side_effect=NotFound())

client = WorkspaceClient(config=config)
client.dbfs.exists('/abc/def/ghi')

get_status.assert_called_with('/abc/def/ghi')


def test_volume_exists(config, mocker):
from databricks.sdk import WorkspaceClient

get_metadata = mocker.patch('databricks.sdk.service.files.FilesAPI.get_metadata')

client = WorkspaceClient(config=config)
client.dbfs.exists('/Volumes/abc/def/ghi')

get_metadata.assert_called_with('/Volumes/abc/def/ghi')

0 comments on commit f0ac1da

Please sign in to comment.