Skip to content

fs: merge upload_fobj and upload #6570

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

Merged
merged 2 commits into from
Sep 9, 2021
Merged

Conversation

skshetry
Copy link
Collaborator

@skshetry skshetry commented Sep 9, 2021

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

@skshetry skshetry requested a review from a team as a code owner September 9, 2021 06:25
@efiop
Copy link
Contributor

efiop commented Sep 9, 2021

@skshetry Btw, could you clarify how you see it in relation to fsspec?

@skshetry
Copy link
Collaborator Author

skshetry commented Sep 9, 2021

I don't think I have any motivations to unify them from fsspec perspective. My first preference there would be to have an explicit API like upload_fileobj/upload_fobj and my second choice would be to extend pipe_file to support file objects.
upload and upload_fobj are quite different things in the filesystem layer.

For us, what we need is a fs.utils.write like method (discussed in #6558 (comment)) that can also provide callback support, which is more of a service layer for fs.

I am assuming upload_fobj as a fsspec's API at least for now, so I have tried to not add callback support there, as the caller can do so themselves quite easily.

I'll consider upload API to be that fs.utils.write, that extends fsspec API for us, rather than being fsspec-related.

@efiop efiop merged commit 4bd6d9f into iterative:master Sep 9, 2021
@skshetry skshetry deleted the merge-upload-fobj branch September 9, 2021 10:09
@efiop efiop added refactoring Factoring and re-factoring skip-changelog Skips changelog labels Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Factoring and re-factoring skip-changelog Skips changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants