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

ZipFS has a syspath and ospath #528

Closed
tfeldmann opened this issue Mar 21, 2022 · 5 comments
Closed

ZipFS has a syspath and ospath #528

tfeldmann opened this issue Mar 21, 2022 · 5 comments

Comments

@tfeldmann
Copy link
Contributor

tfeldmann commented Mar 21, 2022

As discussed in #523, normally ZipFS should have no syspath and a NoSysPath exception should be raised.

>>> import fs
>>> x = fs.open_fs("zip://test.zip", create=True)
>>> x.getospath("/")
b'/var/folders/lt/sg8874fn2lz5y3wcphs91j440000gn/T/tmpvdg2plgy__ziptemp__/'
>>> x.getsyspath("/")
'/var/folders/lt/sg8874fn2lz5y3wcphs91j440000gn/T/tmpvdg2plgy__ziptemp__/'
@lurch
Copy link
Contributor

lurch commented Mar 21, 2022

Do you fancy submitting a suitable PR?

@tfeldmann
Copy link
Contributor Author

Sure, no problem.

@althonos
Copy link
Member

althonos commented Mar 22, 2022

Ah, okay, I see what i was talking about in #523, but I'm not entirely sure this current situation here is to be treated as a bug.

When you open a Zip file, you get a ReadZipFS which doesn't have a syspath because files are decompressed on request. But when you create a new Zip file from scratch, you get a WriteZipFS, which is actually a simple wrapper around a temporary filesystem which will be compressed when closed.

So if the temporary filesystem is a temp folder, it makes sense that you have a syspath. It also enables efficient copying / moving files that are going to be compressed. There will be no syspath in the event where you use a temporary filesystem in memory to prepare the archive:

zip_fs = fs.zipfs.WriteZipFS("/tmp/test.zip", temp_fs="mem://")

@tfeldmann
Copy link
Contributor Author

I see! Then the question is what went wrong with ZipFS in my optimization tests. I'll try to reproduce it.

@tfeldmann
Copy link
Contributor Author

Closing this as a non-issue. ZipFS having a syspath when wrapping a temp filesystem is fine.

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

No branches or pull requests

3 participants