Add VFS groundwork #5
Conversation
| def __init__(self, parent_file, zipfile, zipinfo): | ||
| self.parent_file = parent_file | ||
| self.zipfile = zipfile | ||
| self.zipinfo = zipinfo |
There was a problem hiding this comment.
could validate that pieces that are required are given and are of valid type/interface
I would also add the input types to these constructors (I can find that parent file is Optional["File"], which is
untrue as it is required in this context
e.g. missing parent_file will cause a crash when .name is read
| return os.path.join(os.path.dirname(self.parent_file.name), self.tarinfo.path) | ||
|
|
||
|
|
||
| def find_files_in_zip(vr: "VFS", df: FileOnDisk) -> None: |
There was a problem hiding this comment.
I would have all of the classes and some of the functions in separate files under a vfs module but that is just a personal preference
| ) | ||
|
|
||
|
|
||
| class VFS: |
There was a problem hiding this comment.
why abbreviate this? especially as it isn't explained what it means and you always assign it to a variable anyway
would name it ValohaiFileSystem (I guess this is what it means btw as I don't know); could also be VirtualFileSystem?
hmm, and naming it ValohaiFileSystem reduces it's usability outside of Valohai context; do we want this to potentially be usable open source library in it's own right also? so I would name it VirtualFileSystem or something like that if that is the case
|
I will merge this as is and comments are evaluated and acted upon in another pull request. |
No description provided.