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

Misleading API function names download_file and upload_file #57

Open
thomasborgen opened this issue Mar 30, 2022 · 1 comment
Open

Misleading API function names download_file and upload_file #57

thomasborgen opened this issue Mar 30, 2022 · 1 comment
Assignees
Labels
[semver:major] Use [semver:major] in merge message

Comments

@thomasborgen
Copy link
Owner

Is your feature request related to a problem? Please describe.

download_file and upload_file function names are misleading.

The problem with download_file is that while it does consume a storage bucket blob, it does not write to a file, it just returns bytes.
The problem with upload_file is that from the name one would expect the input to the function to be a filename, but it is actually bytes.

Describe the solution you'd like

Clearer api.

upload - input data: str | bytes
upload_from_file - input filename: str

download - returns str | bytes
download_to_file - return filename: str or absolute path to the file.

or have string and bytes spesific functions as well. but that might be a bit overkill.

@thomasborgen
Copy link
Owner Author

Maybe its not that straight forward.
We decided not to call the blobs in storage bucket for blobs, but for files. Which kind of led to the names upload_file and download file.

Not a 100% sure how to move forward. My issue when i looked into changing it was this:

def download(storage_bucket_name, filename): ...

def download_to_file(storage_bucket_name, filename, local_filename): ...

The argument naming gets a bit weird 😓

So then maybe we should change the argument names aswell. But if we do that we should create a consistent naming standard and we should probably just follow what google does... Calling a blob a blob is probably just the easiest way forward where a blob is data in a storage bucket and a file is data stored in a conventional way.

@thomasborgen thomasborgen self-assigned this Mar 31, 2022
@thomasborgen thomasborgen added the [semver:major] Use [semver:major] in merge message label Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[semver:major] Use [semver:major] in merge message
Projects
None yet
Development

No branches or pull requests

1 participant