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

How to use local Unix paths with Pathy #87

Closed
b2m opened this issue Aug 8, 2022 · 4 comments · Fixed by #95
Closed

How to use local Unix paths with Pathy #87

b2m opened this issue Aug 8, 2022 · 4 comments · Fixed by #95
Labels

Comments

@b2m
Copy link

b2m commented Aug 8, 2022

Context

spaCy is describing on it's website that it can store project data either locally or remotely using Pathy.

Example Paths from the spaCy's documentation:

  • s3://my-spacy-bucket
  • /mnt/scratch/cache
  • ssh://myserver.example.com/whatever

But using local paths like /mnt/scratch/cache does not work.

Problem

Using a local Unix path currently seems not to be supported by Pathy.
I modified the example code from the project's README;

from pathy import Pathy
# Create a bucket
Pathy("/tmp/test").mkdir(exist_ok=True)
# An excellent blob
greeting = Pathy("/tmp/test/greeting.txt")
# But it doesn't exist yet
assert not greeting.exists()
# Create it by writing some text
greeting.write_text("Hello World!")
# Now it exists
assert greeting.exists()
# Delete it
greeting.unlink()
# Now it doesn't
assert not greeting.exists()

This will result in a PermissionError: [Errno 13] Permission denied: '/test'.
Note the missing /tmp part in the path!

  Traceback (most recent call last):
  File "test_pathy.py", line 9, in <module>
    greeting.write_text("Hello World!")
  File "/usr/lib/python3.8/pathlib.py", line 1255, in write_text
    with self.open(mode='w', encoding=encoding, errors=errors) as f:
  File "/pathy_test/venv/lib/python3.8/site-packages/pathy/__init__.py", line 672, in open
    return self._accessor.open(
  File "/pathy_test/venv/lib/python3.8/site-packages/pathy/__init__.py", line 412, in open
    return self.client(path).open(
  File "/pathy_test/venv/lib/python3.8/site-packages/pathy/__init__.py", line 1001, in open
    full_path.mkdir(parents=True, exist_ok=True)
  File "/usr/lib/python3.8/pathlib.py", line 1288, in mkdir
    self._accessor.mkdir(self, mode)
PermissionError: [Errno 13] Permission denied: '/test'

So I am wondering, whether using Pathy('/some/unix/path') should be discouraged in favor of Pathy.fluid('/some/unix/path') or whether this is a bug in Pathy.

Possibly related issues:

@adrianeboyd
Copy link
Contributor

This is a bug in spacy. I ran into the same issue when trying to use local remote storage for testing, see explosion/spaCy#11762.

@justindujardin
Copy link
Owner

Sorry for the late reply, @b2m, you should use Pathy.fluid when you are working with a path of an unknown type. You could alternatively use the pathlib.Path class if you want to explicitly represent a unix path.

justindujardin added a commit that referenced this issue Nov 18, 2022
 - this addresses concerns around initializing Pathy paths with unix and windows absolute paths. The correct way to initialize these paths is to use Pathy.fluid.
 - raise an error if an absolute system path is given when initializing Pathy objects
 - fixes #87

BREAKING CHANGE: Previously Pathy would allow you to initialize Pathy instances with absolute system paths (unix and windows). Now Pathy raises a ValueError if given an absolute system path that suggest using Pathy.fluid instead.
@justindujardin
Copy link
Owner

I've thought about this some more, and I think there should be an error if you initialize Pathy with Unix paths instead of using Pathy.fluid. This should resolve your problem and keep people from running into the same problem in the future.

Changes coming in #95

github-actions bot pushed a commit that referenced this issue Nov 22, 2022
# [0.9.0](v0.8.1...v0.9.0) (2022-11-22)

### Bug Fixes

* **blob:** properly initialize default last_modified ([d831bee](d831bee))
* **windows:** consistent path separator in resolve ([44f5ca0](44f5ca0))
* **windows:** file:/// paths had the wrong suffix ([674a109](674a109))
* **windows:** return None owner on windows where not implemented ([abd28c4](abd28c4))

### Features

* **Pathy:** raise error when not using Pathy.fluid for absolute paths ([e7f4e73](e7f4e73)), closes [#87](#87)
* **windows:** add windows CI test execution ([504823d](504823d))

### BREAKING CHANGES

* **Pathy:** Previously Pathy would allow you to initialize Pathy instances with absolute system paths (unix and windows). Now Pathy raises a ValueError if given an absolute system path that suggest using Pathy.fluid instead.
@github-actions
Copy link

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants