Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise a TypeError if wrong type given to fix_url #2936

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/python/quilt3/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ def set_dir(self, lkey, path=None, meta=None, update_policy="incoming"):
Args:
lkey(string): prefix to add to every logical key,
use '/' for the root of the package.
path(string): path to scan for files to add to package.
path(string, os.PathLike): path to scan for files to add to package.
If None, lkey will be substituted in as the path.
meta(dict): user level metadata dict to attach to lkey directory entry.
update_policy(str): can be either 'incoming' (default) or 'existing'.
Expand Down
11 changes: 8 additions & 3 deletions api/python/quilt3/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,18 @@ def __str__(self):

def fix_url(url):
"""Convert non-URL paths to file:// URLs"""
# If it has a scheme, we assume it's a URL.
# On Windows, we ignore schemes that look like drive letters, e.g. C:/users/foo
# We could technically skip this check and use the TypeError check alone,
# but there are existing tests that expect a ValueError for NoneType objects
if not url:
raise ValueError("Empty URL")
if not isinstance(url, (str, os.PathLike)):
raise TypeError(f"Expected a string or pathlike object, but got an instance of {type(url)}.")

url = str(url)
if isinstance(url, os.PathLike):
url = url.__fspath__()
eode marked this conversation as resolved.
Show resolved Hide resolved

# If it has a scheme, we assume it's a URL.
# On Windows, we ignore schemes that look like drive letters, e.g. C:/users/foo
parsed = urlparse(url)
if parsed.scheme and not os.path.splitdrive(url)[0]:
return url
Expand Down
7 changes: 7 additions & 0 deletions api/python/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@


# Code
def test_fix_url():
with pytest.raises(TypeError):
util.fix_url(object())
with pytest.raises(ValueError):
util.fix_url(None)


def test_write_yaml(tmpdir):
fname = tmpdir / 'some_file.yml'

Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference/Package.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ __Arguments__

* __lkey(string)__: prefix to add to every logical key,
use '/' for the root of the package.
* __path(string)__: path to scan for files to add to package.
* __path(string, os.PathLike)__: path to scan for files to add to package.
If None, lkey will be substituted in as the path.
* __meta(dict)__: user level metadata dict to attach to lkey directory entry.
* __update_policy(str)__: can be either 'incoming' (default) or 'existing'.
Expand Down