-
Notifications
You must be signed in to change notification settings - Fork 57
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
[multipart] Support bytes[]/content type
#2380
Changes from 9 commits
94e6e3d
a45a47c
575568f
b8fd3ff
97932ba
ca46a17
44747f3
fba4a91
f7d4ce6
f0eb959
7280ce8
a847978
6493106
3d78277
fbc88e6
e72641e
5518329
9c4db12
445d4a3
3e57197
3e70bac
913c3f5
34d9725
915f34b
7062043
4a887a1
b41e539
ca66468
8734335
fe19141
0375e3d
1d977fd
8f53d03
0da11ba
7bfd08f
6704313
dcf59fa
3eb1f38
2c256e6
6d3da87
2533326
c15d6b4
43520d4
e8d1b7a
ad3174d
9731549
e49b726
7ad0dbb
a7a41c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,9 @@ | |
from .constant_type import ConstantType | ||
from .utils import add_to_description | ||
from .combined_type import CombinedType | ||
from .model_type import JSONModelType | ||
from .model_type import JSONModelType, DPGModelType | ||
from .primitive_types import ByteArraySchema | ||
from .list_type import ListType | ||
|
||
if TYPE_CHECKING: | ||
from .code_model import CodeModel | ||
|
@@ -278,6 +280,25 @@ def has_json_model_type(self) -> bool: | |
return self.type.target_model_subtype((JSONModelType,)) is not None | ||
return isinstance(self.type, JSONModelType) | ||
|
||
@property | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also we can actually support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before #2380 (comment), it passes |
||
def file_properties(self) -> List[str]: | ||
model_type = None | ||
if isinstance(self.type, CombinedType): | ||
model_type = self.type.target_model_subtype((JSONModelType, DPGModelType)) | ||
elif isinstance(self.type, (JSONModelType, DPGModelType)): | ||
model_type = self.type | ||
if model_type is None: | ||
return [] | ||
return [ | ||
prop.wire_name | ||
for prop in model_type.properties | ||
if isinstance(prop.type, ByteArraySchema) | ||
or ( | ||
isinstance(prop.type, ListType) | ||
and isinstance(prop.type.element_type, ByteArraySchema) | ||
) | ||
] | ||
|
||
@classmethod | ||
def from_yaml( | ||
cls, yaml_data: Dict[str, Any], code_model: "CodeModel" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,10 +5,10 @@ | |
# Changes may cause incorrect behavior and will be lost if the code is regenerated. | ||
# -------------------------------------------------------------------------- | ||
|
||
from io import BytesIO, IOBase | ||
from io import IOBase | ||
import json | ||
import sys | ||
from typing import Any, Union | ||
from typing import Any, List, Tuple, Union | ||
import uuid | ||
|
||
from ._model_base import Model, SdkJSONEncoder | ||
|
@@ -19,16 +19,19 @@ | |
from typing import MutableMapping # type: ignore # pylint: disable=ungrouped-imports | ||
|
||
|
||
class NamedBytesIO(BytesIO): | ||
def __init__(self, name: str, *args, **kwargs): | ||
super().__init__(*args, **kwargs) | ||
self.name = name | ||
JSON = MutableMapping[str, Any] # pylint: disable=unsubscriptable-object | ||
|
||
# file-like tuple could be `(filename, IO (or bytes))` or `(filename, IO (or bytes), content_type)` | ||
FileType = Union[IOBase, bytes] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use the type hints I have here from core https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/core/azure-core/azure/core/rest/_helpers.py#L79 |
||
FileTuple = Union[Tuple[str, FileType], Tuple[str, FileType, str]] | ||
MultiPartFile = Union[IOBase, bytes, FileTuple] | ||
HandledMultiPartFile = Union[IOBase, FileTuple] | ||
|
||
def multipart_file(file: Union[IOBase, bytes]) -> IOBase: | ||
if isinstance(file, IOBase): | ||
|
||
def multipart_file(file: MultiPartFile) -> HandledMultiPartFile: | ||
if isinstance(file, (IOBase, tuple)): | ||
return file | ||
return NamedBytesIO("auto-name-" + str(uuid.uuid4()), file) | ||
return ("auto-name-" + str(uuid.uuid4()), file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we giving auto names to these file fields? That's def not what we should be doing. We can get the field name from the typespec and then create a tuple of all of the So class MyMultipartModel:
profile_image: MultipartFile = rest_field("profileImage")
pictures: MultipartFile[] = rest_field("pictures")
def my_multipart_func(body: MyMultipartModel):
_files = [("profileImage", body.profile_image)]
_files.extend([("pictures", p)] for p in body.pictures)
request = HttpRequest(..., files=_files)
... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Current logic is actually similar to yours and it also compatible when body type is JSON There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SHOULD doesn't mean MUST. If the IO is not a file descriptor and we don't have a name, we don't set it. No autoname, this will be sent to the servirce, that may save it, or even act based on the received name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading the code more, this function should not exist |
||
|
||
|
||
def multipart_data(data: Any) -> Any: | ||
|
@@ -37,21 +40,48 @@ def multipart_data(data: Any) -> Any: | |
return data | ||
|
||
|
||
def handle_multipart_form_data_model(body: Model) -> MutableMapping[str, Any]: # pylint: disable=unsubscriptable-object | ||
def handle_multipart_form_data_model( | ||
body: Model, file_properties: List[str] | ||
) -> JSON: # pylint: disable=unsubscriptable-object | ||
"""handle first layer of model. | ||
If its value is bytes or IO, replace it with raw value instead of serialized value. | ||
|
||
:param body: The model to handle. | ||
:type body: ~payload.multipart._model_base.Model | ||
:type body: ._model_base.Model | ||
:param file_properties: The properties of the model that are file type. | ||
:type file_properties: list[str] | ||
:return: The handled model. | ||
:rtype: MutableMapping[str, Any] | ||
""" | ||
result = body.as_dict() | ||
rest_name_attr = {v._rest_name: k for k, v in body._attr_to_rest_field.items()} # pylint: disable=protected-access | ||
for rest_name in result.keys(): | ||
attr = rest_name_attr.get(rest_name) | ||
if attr is not None: | ||
raw_value = getattr(body, attr, None) | ||
if isinstance(raw_value, (bytes, IOBase)): | ||
result[rest_name] = raw_value | ||
if attr is not None and rest_name in file_properties: | ||
result[rest_name] = getattr(body, attr, None) | ||
return result | ||
|
||
|
||
def handle_multipart_form_data_body( | ||
body: Union[Model, JSON], file_properties: List[str] | ||
) -> List[Tuple[str, HandledMultiPartFile]]: | ||
"""handle multipart form data body. | ||
|
||
:param body: The body to handle. | ||
:type body: ._model_base.Model or dict[str, Any] | ||
:param file_properties: The properties of the model that are file type. | ||
:type file_properties: list[str] | ||
:return: The handled body. | ||
:rtype: list[tuple[str, HandledMultiPartFile]] | ||
""" | ||
_body = handle_multipart_form_data_model(body, file_properties) if isinstance(body, Model) else body | ||
files = [] | ||
for field_name, value in _body.items(): | ||
if field_name in file_properties: | ||
if isinstance(value, list): | ||
files.extend([(field_name, multipart_file(i)) for i in value]) | ||
else: | ||
files.append((field_name, multipart_file(value))) | ||
else: | ||
files.append((field_name, multipart_data(value))) | ||
return files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wit this dpg PR Azure/typespec-azure#166 all we have to do is create a new type
MultipartFileType
and then type this type asUnion[binary, Tuple[string, binary], Tuple[string, binary, string]]