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

pyFilesystem support for loading and saving data #205

Closed
jayvdb opened this issue Jan 29, 2019 · 14 comments
Closed

pyFilesystem support for loading and saving data #205

jayvdb opened this issue Jan 29, 2019 · 14 comments
Milestone

Comments

@jayvdb
Copy link
Member

jayvdb commented Jan 29, 2019

https://github.com/PyFilesystem

moban currently supports custom filesystem 'git'.

To add more it should adopt a generic fs layer, like hyde.

@chfw
Copy link
Member

chfw commented Jan 29, 2019

does it mean wherever moban uses file system, use pyFilesystem?

@jayvdb
Copy link
Member Author

jayvdb commented Jan 29, 2019

possible, but not a good idea imo.

Also pyfs doesnt support git.

this is only for non-local files, so e.g. deployment could be done with moban.

@jayvdb
Copy link
Member Author

jayvdb commented Jul 12, 2019

A github driver is at https://github.com/merlink01/fs.github . Not particularly useful as it doesnt support GitLab, but may be use in addition to a git driver, if one is created.

@jayvdb
Copy link
Member Author

jayvdb commented Jul 13, 2019

When we do switch, there is a jinja template loader, https://github.com/althonos/jinja2-fsloader

@jayvdb
Copy link
Member Author

jayvdb commented Jul 13, 2019

We could take the Hyde approach and have one custom driver for git, and use pyfs for everything else, and transparently wrap the git repo in a pyfs layer.

@jayvdb
Copy link
Member Author

jayvdb commented Jul 13, 2019

Created merlink01/fs.github#9 to see if they are interested in GitLab/etc support.

If not, we may need to create a fs.IGitt ourselves.

@chfw
Copy link
Member

chfw commented Jul 19, 2019

do we want to expose pyfilesystem2 syntax to our users(downstream projects)?

@jayvdb
Copy link
Member Author

jayvdb commented Jul 20, 2019

Optional, but yes. Maybe even recommended where a value might be ambiguous without a pyfilesystem2 prefix. It allows users to host their config & templates in any filesystem-like provider, and use a pyflesystem2 URL to tell moban where it is. This is a natural way to say "moban supports any data/file host" without adding/maintaining drivers for all of them. And we inherit the naming conventions of pyfilesystem which prevents any conflicts/ambiguity, and means moban users can use pyfilesystem docs for precise semantics of the URLs. Our users get new file hosts each time pyfilesystem core includes a new one.

The most obvious "win" is that if we can get a git driver for pyfilesystem, we dont need to create special code for mercurial and other VCS: anyone who wants those can create their own pyfilesystem driver.

The standardisation inside moban codebase will also be beneficial. It's support of PyPI packages would be a simple wrapper that can be creating using the core pyfilesystem2 drivers.(I expect it would be an internal driver, not a separately pypi published one).

@chfw
Copy link
Member

chfw commented Jul 20, 2019

Yes, pyfilesystem2 actually complements our vision with any location and any format. With jinja2_loader, our template could sit on s3.

I am finding a blocker in practice:

moban scans input and forms a list of things to do, for example: here is a template file, a data file, please render an output. Then moban does it.

moban collects absolute file paths into a data structure. then take them out to read or write.

Essentially with pyfilesystem2: we need it to fulfil two fundamental things:

  1. does the file exist?

pseudo code:

with open_fs('pypi://pypi-mobans-pkg/templates') as pypi_fs:
... if pypi_fs.exists('reminder.txt'):
... pypi_fs.abspath('reminder.txt')

  1. can I read/write it with an absolute path

fs.write('pypi://pypi-mobans-pkg/templates/reminder.txt', 'my content', 'my encoding')

My concern is:

a. can fs generate a url for any file?
b. can fs action on a single given url?

@chfw chfw mentioned this issue Jul 21, 2019
@chfw
Copy link
Member

chfw commented Jul 21, 2019

it is funny: fs.errors.InvalidCharsInPath: path 'C:\Users\VssAdministrator\AppData\Local\moban\moban\Cache\repos\pypi-mobans\config\data.yml' contains invalid characters

https://dev.azure.com/moremoban/moban/_build/results?buildId=484

@chfw
Copy link
Member

chfw commented Jul 21, 2019

on python 2: TypeError: paths must be unicode (not str) <-- well, python file system 2 said path should be unicode.

https://travis-ci.org/moremoban/moban/jobs/561675011

@chfw
Copy link
Member

chfw commented Jul 26, 2019

@jayvdb , two internal openers for pypi and github were done. but it does not cover:

because they are overlapping requires command, which does above two. for consistency, I would love to remove requires and simply use "pypi://my-template-package/relative/path" everywhere in .moban.yaml.

@jayvdb
Copy link
Member Author

jayvdb commented Jul 27, 2019

I think that is ok, as long as it is a new minor release.

I would prefer to keep requires: because it allows aliases which keep the length of references shorter, but I do agree that using full pyfs urls everywhere is very attractive.

We could re-add requires: -like functionality later, possibly by using mountfs and/or multifs.

I'll review the PR #302 later today.

@chfw
Copy link
Member

chfw commented Aug 17, 2019

the support is to be released in v0.6.0

@chfw chfw closed this as completed Aug 17, 2019
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

2 participants