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