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

http: allow put_file()ing file-like objects #764

Merged
merged 7 commits into from
Sep 22, 2021

Conversation

isidentical
Copy link
Member

Since writing to HTTPFile is not supported, currently you can not upload arbitrary streams (unlike other filesystems where you could open a new file in writing mode through fs.open(path, 'wb') and copy the original stream through shutil.copy_file_obj() to the new file). Not sure if this would be worth documenting at the moment, but would be extremely useful to compensate for the missing w method at the moment while we are thinking about other solutions.

@martindurant
Copy link
Member

This seems like it might be a useful thing to do for all filesystems.

Possibly related: #723

@isidentical
Copy link
Member Author

I agree. It might require a bigger design discussion, but something I wondered when I started to the migration to the fsspec was why this was not supported natively (since that would potentially render the requirement to the callbacks redundant, since we could simply wrap read() or write() methods of the original stream with the callback updates).

@martindurant
Copy link
Member

martindurant commented Sep 21, 2021

Should be callback.set_size(getattr(f, "size", None)) ? Or if hasattr(f, "size"):

@@ -231,21 +232,21 @@ async def _cat_file(self, url, start=None, end=None, **kwargs):
return out

async def _get_file(
Copy link
Member

@martindurant martindurant Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one was right!
It says, "get file from remote to local", and should be the revers of put_file. That's how we have it in spec.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed that! Thanks for the correction.

@martindurant martindurant merged commit b0ce401 into fsspec:master Sep 22, 2021
efiop added a commit to efiop/filesystem_spec that referenced this pull request Dec 10, 2021
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

Successfully merging this pull request may close these issues.

2 participants