Add multipart support to the CreateEtchPacket mutation#41
Conversation
| def get_files_for_payload(self) -> Tuple[List[AttachableEtchFile], List[Any]]: | ||
| """Scan through and gather files for use in multipart requests. | ||
|
|
||
| During the process, files will be replaced with a dummy `Path` | ||
| instance as `Callable` instances and likely their returns of some | ||
| sort of IO reader is not serializable and not easily dealt with in | ||
| pydantic (the underlying model system for the types used in this | ||
| library). Closer to the actual request, when the multipart payload is | ||
| being constructed, the dummy instances will be replaced by their actual | ||
| file instances. | ||
|
|
||
| :return: Tuple containing a list of uploadable file types, and a list | ||
| of actual file references | ||
| :rtype: Tuple[List, List] | ||
| """ | ||
| contains_uploads = any([isinstance(f, DocumentUpload) for f in self.files]) | ||
| if not contains_uploads: | ||
| return self.files, [] | ||
|
|
||
| files = [] | ||
| for idx, f in enumerate(self.files): | ||
| if not isinstance(f, DocumentUpload): | ||
| continue | ||
|
|
||
| if getattr(f.Config, "contains_uploads", False): | ||
| attr_name = getattr(f.Config, "contains_uploads") | ||
| upload = getattr(f, attr_name) | ||
|
|
||
| if not isinstance(upload, Base64Upload): | ||
| files.append(upload) | ||
| setattr(f, attr_name, Path()) | ||
|
|
||
| return self.files, files |
There was a problem hiding this comment.
Admittedly, this is a little funky, but it's the easiest way I can think of at the moment. Because of the multipart spec, we need to a way to check for files, but the file attribute also has to be nulled/None-ed out when we serialize this down to JSON. Because of this, I'm just using a Path instance as a marker since a multipart file can be a file path or function.
| @@ -0,0 +1,34 @@ | |||
| from python_anvil.multipart_helpers import get_extractable_files_from_payload | |||
There was a problem hiding this comment.
Adding more tests shortly..
|
|
||
| files = get_multipart_payload(mutation) | ||
|
|
||
| res = self.mutate_multipart(files, **kwargs) |
There was a problem hiding this comment.
A few things:
- This looks like it will always call
mutatate_multipart, even if there are no binary uploads, and therfore no reason to usemulti-part. Is that right? Maybe only usemulti-partwhen necessary? - It feels like this decision to use multipart or not, and the few lines hre of what to do in that case are better abstracted away inside the internals of
self.mutatesomehow. That way anything that is uploading binary in a mutation will just have that stuff handled implicitly.
| return self.file_payloads | ||
|
|
||
| def create_payload(self) -> CreateEtchPacketPayload: | ||
| def get_files_for_payload(self) -> Tuple[List[AttachableEtchFile], List[Any]]: |
There was a problem hiding this comment.
This also feels like something that belongs in a more shared, non-CreateEtchPacket specific spot.
This seem seems akin to https://www.npmjs.com/package/extract-files
Our node-anvil library has a decent pattern to follow where it figures out if there files to map here and then makes a multi-part request if necessary here.
There was a problem hiding this comment.
The general extract-files-like functionality is in the function here: https://github.com/anvilco/python-anvil/pull/41/files#diff-615974429dd7f7e34ce2e08e0012206ae6f47d86bcfc9338e6f72546e76436beR14
In this first pass, though, this is mainly to add multipart support on createEtchPacket since it's the only one that has underlying model support. Will have a more generic multipart mutation path after this PR.
2 participants
Adds multipart support for the
createEtchPacketmutation via the spec here: https://github.com/jaydenseric/graphql-multipart-request-spec