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

nullable int model parameter treated as a file in multipart with 0.17.0, causing httpx AttributeError #926

Closed
johnthagen opened this issue Jan 2, 2024 · 13 comments · Fixed by #1032

Comments

@johnthagen
Copy link
Collaborator

johnthagen commented Jan 2, 2024

Describe the bug

After upgrading the client to 0.17.0 and testing out, I noticed that Multipart client handling seems to have broken.

The code in 0.16.0 was rougly:

        api.sync_detailed(
            id,
            client=client,
            multipart_data=TRequest(parent=2),
        )

And with 0.17.0, usage updated to:

        api.sync_detailed(
            id,
            client=client,
            body=TRequest(parent=2),
        )

Is this not actually being serialized to multipart at some point? The type signatures all matched, so I thought I was using it correctly. I also see in the generated sync_detailed() that _get_kwargs() calls _body = body.to_multipart() so is it right to assume that multipart should happen transparently?

The model in question has parameters of type, File, str, bool, and int.

I was hoping the stacktrace might help illuminate what could be happening:

    api.sync_detailed(
  File "main.py", line 92, in sync_detailed
    response = client.get_httpx_client().request(
  File "/python3.10/site-packages/httpx/_client.py", line 828, in request
    return self.send(request, auth=auth, follow_redirects=follow_redirects)
  File "/python3.10/site-packages/httpx/_client.py", line 915, in send
    response = self._send_handling_auth(
  File "/python3.10/site-packages/httpx/_client.py", line 943, in _send_handling_auth
    response = self._send_handling_redirects(
  File "/python3.10/site-packages/httpx/_client.py", line 980, in _send_handling_redirects
    response = self._send_single_request(request)
  File "/python3.10/site-packages/httpx/_client.py", line 1016, in _send_single_request
    response = transport.handle_request(request)
  File "/python3.10/site-packages/httpx/_transports/default.py", line 231, in handle_request
    resp = self._pool.handle_request(req)
  File "/python3.10/site-packages/httpcore/_sync/connection_pool.py", line 268, in handle_request
    raise exc
  File "/python3.10/site-packages/httpcore/_sync/connection_pool.py", line 251, in handle_request
    response = connection.handle_request(request)
  File "/python3.10/site-packages/httpcore/_sync/connection.py", line 103, in handle_request
    return self._connection.handle_request(request)
  File "/python3.10/site-packages/httpcore/_sync/http11.py", line 133, in handle_request
    raise exc
  File "/python3.10/site-packages/httpcore/_sync/http11.py", line 94, in handle_request
    self._send_request_body(**kwargs)
  File "/python3.10/site-packages/httpcore/_sync/http11.py", line 154, in _send_request_body
    for chunk in request.stream:
  File "/python3.10/site-packages/httpx/_multipart.py", line 264, in __iter__
    for chunk in self.iter_chunks():
  File "/python3.10/site-packages/httpx/_multipart.py", line 230, in iter_chunks
    yield from field.render()
  File "/python3.10/site-packages/httpx/_multipart.py", line 190, in render
    yield from self.render_data()
  File "/python3.10/site-packages/httpx/_multipart.py", line 183, in render_data
    chunk = self.file.read(self.CHUNK_SIZE)
AttributeError: 'int' object has no attribute 'read'. Did you mean: 'real'?

OpenAPI Spec File

(Minimimized)

  /t/{id}/:
    patch:
      operationId: 
      description: |-
        T
      parameters:
      - in: path
        name: id
        schema:
          type: integer
        description: 
        required: true
      requestBody:
        content:
          multipart/form-data:
            schema:
              $ref: '#/components/schemas/TRequest'
      security:
      - jwtAuth: []
      - basicAuth: []
      - cookieAuth: []
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TModel'
          description: ''
components:
  schemas:
    TRequest:
      type: object
      properties:
        file:
          type: string
          format: binary
        label:
          type: string
          minLength: 1
        flag:
          type: boolean
        project:
          type: integer
        parent:
          type: integer
          nullable: true

Desktop:

  • OS: macOS 13.6
  • Python Version: 3.10.13
  • openapi-python-client version: 0.17.0

Additional context

  • httpx 0.26.0
  • OpenAPI schema 3.0.3
@johnthagen
Copy link
Collaborator Author

johnthagen commented Jan 3, 2024

Looking into this some more, the model I'm generating looks like:

@_attrs_define
class TRequest:
    file: Union[Unset, File] = UNSET
    parent: Union[None, Unset, int] = UNSET
    ...

parent is a primary key, so it's a simple int. But the new _get_kwargs generates

def _get_kwargs(
    id: int,
    *,
    body: TRequest,
) -> Dict[str, Any]:
    headers: Dict[str, Any] = {}

    _kwargs: Dict[str, Any] = {
        "method": "patch",
        "url": "/t/{id}/".format(
            id=id,
        ),
    }

    _body = body.to_multipart()

    _kwargs["files"] = _body

    _kwargs["headers"] = headers
    return _kwargs

When called like:

sync_detailed(
    1,
    client=client,
    body=TRequest(parent=2),
 )

This generates a kwargs in sync_detailed() that looks like:

{
  'files': {'parent': 4}, 
  'headers': {}, 
  'method': 'patch', 
  'url': '/t/2/'
}

And then it seems like the parent primary key int is being passed to httpx as if it were a File? Thus why httpx is trying to run read on it.

@johnthagen
Copy link
Collaborator Author

johnthagen commented Jan 3, 2024

In contrast, the 0.16.0 created these kwargs which do work properly (passed through httpx and when processed by the server):

{
    "files": {"parent": (None, b"6", "text/plain")},
    "method": "patch",
    "url": "/t/2/",
}

@johnthagen johnthagen changed the title httpx multipart AttributeError generating client with 0.17.0 int model parameter treated as a file in multipart with 0.17.0, causing httpx AttributeError Jan 3, 2024
@johnthagen
Copy link
Collaborator Author

I confirmed that this bug is still present in 0.17.2.

@johnthagen
Copy link
Collaborator Author

The regression seems to have come from

@johnthagen
Copy link
Collaborator Author

@dbanty Do you have any thoughts here? I think I've traced the issue about as far as I can. Thanks.

@dbanty
Copy link
Collaborator

dbanty commented Jan 23, 2024

@johnthagen any chance that #938 fixes this for you? I haven't had a chance to test it out, but your validation would be enough for me!

@johnthagen
Copy link
Collaborator Author

johnthagen commented Jan 24, 2024

@dbanty I tried this:

pip install git+https://github.com/openapi-generators/openapi-python-client.git@refs/pull/938/merge

from #938 and regenerated the client. I did see some changes to the generated to_multipart() methods in a number of my models, but the problem listed in this issue is still present.

@johnthagen
Copy link
Collaborator Author

johnthagen commented Jan 24, 2024

I drilled down into the diff between 0.16.0 and 0.17.x again some more, I found the actual line that is causing the problem.

In 0.16.0, within to_multipart(), the line for parent was:

        parent = self.parent if isinstance(self.parent, Unset) else (None, str(self.parent).encode(), "text/plain")

But in 0.17.x it is changed to:

        parent: Union[None, Unset, int]
        if isinstance(self.parent, Unset):
            parent = UNSET
        else:
            parent = self.parent

And thus, this int isn't being properly encoded as other ints in the model are (the non-nullable ones are still being encoded as "text/plain").

Is there perhaps a logic bug because parent is nullable?

@johnthagen johnthagen changed the title int model parameter treated as a file in multipart with 0.17.0, causing httpx AttributeError nullable int model parameter treated as a file in multipart with 0.17.0, causing httpx AttributeError Jan 24, 2024
@johnthagen
Copy link
Collaborator Author

@dbanty Curious if you had had any chance to look into the tracing down in the comment above?

@dbanty
Copy link
Collaborator

dbanty commented Mar 7, 2024

Yeah, I think the answer is quite messy and is going to require a lot of template logic. I think the right way to fix it is to completely separate out the json vs multipart template logic instead of continuing to try and reuse bits and pieces, since there are also weird things right now like checking if a value is unset even if it's required.

@dbanty
Copy link
Collaborator

dbanty commented Mar 9, 2024

@johnthagen when you get a chance, please check out #995 to see if that's closer to expected. I still have to do some cleanup, so it won't be perfect

@johnthagen
Copy link
Collaborator Author

johnthagen commented Mar 11, 2024

I first double checked that 0.19.0 stable release still causes this issue, and, as expected, it does.

@johnthagen
Copy link
Collaborator Author

johnthagen commented Mar 11, 2024

@dbanty I installed the PR using:

python -m pip install git+https://github.com/openapi-generators/openapi-python-client.git@refs/pull/995/merge

Regenerated the client, tested my multipart endpoint, and it worked great using this PR! 🚀

dbanty added a commit that referenced this issue May 18, 2024
WIP Fix for #926

---------

Co-authored-by: Dylan Anthony <[email protected]>
dbanty added a commit that referenced this issue May 18, 2024
This PR was created by Knope. Merging it will create a new release

### Breaking Changes

#### `const` values in responses are now validated at runtime

Prior to this version, `const` values returned from servers were assumed
to always be correct. Now, if a server returns
an unexpected value, the client will raise a `ValueError`. This should
enable better usage with `oneOf`.

PR #1024. Thanks @peter-greenatlas!

#### Switch YAML parsing to 1.2

This change switches the YAML parsing library to `ruamel.yaml` which
follows the YAML 1.2 specification.
[There are breaking
changes](https://yaml.readthedocs.io/en/latest/pyyaml/#defaulting-to-yaml-12-support)
from YAML 1.1 to 1.2,
though they will not affect most use cases.

PR #1042 fixes #1041. Thanks @rtaycher!

### Features

- allow Ruff 0.4 (#1031)

### Fixes

#### Fix nullable and required properties in multipart bodies

Fixes #926.

> [!WARNING]
> This change is likely to break custom templates. Multipart body
handling has been completely split from JSON bodies.

Co-authored-by: GitHub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants