graphql-python/gql

Clean up the file upload interface

leszekhanusz opened this issue · 1 comments

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).

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