Implement TarFS.geturl and ZipFS.geturl and Fix #329, #333, #340

[chfw]

Type of changes

• Bug fix
• New feature
• Documentation / docstrings
• Tests
• Other

Checklist

• I've run the latest black with default args on new code.
• I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
• I've added tests for new code.
• I accept that @willmcgugan may be pedantic in the code review.

Description

Fix #329, #333, #340

And deliver geturl for ZipFS and TarFS, and my use case is to get url for a file inside a zip, deposit the url, and read it later.

        with file_system.open_fs('zip://test.zip') as fs_handle:
                template_file_exists = fs_handle.exists(
                    template
                ) and fs_handle.isfile(template)

                if template_file_exists:
                    return fs_handle.geturl(template)

The existing code in ZipFS and TarFS did not imagine an download url is really needed for a file inside a zip or tar. Hence, I would like to present my use case and get it supported.

[chfw]

Hi all, I will get the test cases fixed later

[coveralls]

Coverage decreased (-0.1%) to 98.883% when pulling 15c43d9 on moremoban:master into 84719be on PyFilesystem:master.

[willmcgugan]

@chfw This isn't actually the intent of the geturl method. The URL returned was to be something that could be consumed externally by a browser for example. The URLs you have added are FS URLs which may not be of much use outside of PyFilesystem.

Good catch with the missing archive exceptions.

How did you insert those funky icons in the commit messages? Some tool or do you do them manually?

[chfw]

@willmcgugan here is the fit commit emoji style guide: https://github.com/slashsbin/styleguide-git-commit-message

I accept that your intent was meant for browser. However, I am using zipfs and tarfs in a new context where: I would need a universal url that pyfs2 willingly read a file out. Both tarfs and zipfs do read content out given: zip://test.zip/my/folder/file. But both refuses to emit url which I find it naturally shall emit. FS has its interface specified already hence all sub fs shall respect where applicable. Then we won’t have to remember which fs can and cannot do.

[willmcgugan]

zip://test.zip/my/folder/file

I'd be surprised if this worked as is. I'd expect that it would consider test.zip to be a folder, and it would attempt to open "file" as a zip.

FS URLs do support a syntax that can indicate a path within a filesystem, by separating the filesystem part from the path with a !. Your example could be encoded like this:

>>> from fs.opener.parse import parse_fs_url
>>> parse_fs_url("zip://test.zip!/my/folder/file")
ParseResult(protocol='zip', username=None, password=None, resource='test.zip', params={}, path='/my/folder/file')

The URLs returned must be externally consumable I'm afraid, it could break some projects if we change that. However, there is a purpose parameter to geturl which could be used to request a FS URL. How about the string "fs"?

It would work something like this:

>>> my_zip.geturl("bar/egg", purpose="fs")
"zip://foo.zip!bar/egg"

And that won't break any existing code as purpose defaults to "download"

[chfw]

I think your advice is great! My use case is taken care of and the change won't break others.

"zip://test.zip!/my/folder/file"

Let me update it.

And yes, your doubt is valid. There exists an abstract layer in my lib handles the URL: https://github.com/moremoban/moban/blob/pyfs2/moban/file_system.py#L98

[willmcgugan]

Great work, thanks for sticking at it. Just a few requests.

[lurch]

This PR is getting so big, I wonder if it's worth splitting it into smaller separate PRs? 🤷‍♂️

[chfw]

Or Merge it now and I will prepare another PR to address your review comments.

[chfw]

I have addressed all review comments now. The ball is in your court.

[chfw]

Could you please approve the PR?

[willmcgugan]

@chfw Just want to give it one last look over. Will do that soon.

[chfw]

@willmcgugan Any chance for this PR to get released soon?

[willmcgugan]

Sorry for the delay @chfw All good, but I've merged a PR and created a merge conflict for you. If you wouldn't mind resolving that and we should be good to go.

[chfw]

@willmcgugan , good to merge now.

[chfw]

Hooray!