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

Clean up the file upload interface #437

Open
leszekhanusz opened this issue Sep 13, 2023 · 1 comment
Open

Clean up the file upload interface #437

leszekhanusz opened this issue Sep 13, 2023 · 1 comment
Labels
type: feature A new feature

Comments

@leszekhanusz
Copy link
Collaborator

The file upload functionality of gql allows the user to provide as a file variable either:

  • an opened file
  • a StreamReader instance (for streaming from a download)
  • an async generator (for streaming a local file for example)

but then we added the possibility to

  • change the optional filename property to the name property of the provided object
  • change the content_type property by setting that property on the provided object

The problem is that you cannot set a name to an async generator or change the name property of a file without resorting to hacks.

I propose to change the current interface to require the user to provide as a file variable a new FileVar dataclass which can contain any optional argument we want.

We could then:

  • open files ourselves and close them afterwards
  • add a streaming bool to create the streaming async generator ourselves
  • add filename and content_type optional attributes to change those parameters for any provided

Instead of:

with open("YOUR_FILE_PATH", "rb") as f:

    params = {"file": f}

we would have simply:

params = {"file": FileVar("YOUR_FILE_PATH")}

and changing the filename property would be as simple as:

params = {"file": FileVar("YOUR_FILE_PATH", filename="my_new_filename.txt")}

Also instead of:

async def file_sender(file_name):
    async with aiofiles.open(file_name, 'rb') as f:
        chunk = await f.read(64*1024)
            while chunk:
                yield chunk
                chunk = await f.read(64*1024)

params = {"file": file_sender(file_name='YOUR_FILE_PATH')}

we could have:

params = {"file": FileVar("YOUR_FILE_PATH", streaming=True)}

(while still allowing custom async generators of course for more advanced streaming options)

the FileVar dataclass would look something like this:

@dataclass
class FileVar:
    f: str | io.IOBase | aiohttp.StreamReader | AsyncGenerator
    _: KW_ONLY
    filename: Optional[str] = None
    content_type: Optional[str] = None
    streaming: bool = False
    streaming_block_size: int = 64*1024

This would be a breaking change but we could still allow the old classes (but add a deprecated warning).

@leszekhanusz leszekhanusz added the type: feature A new feature label Sep 13, 2023
@leszekhanusz
Copy link
Collaborator Author

Note that we would also need to add aiofiles as a dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

No branches or pull requests

1 participant