Skip to content

Attach updated endpoint signature to inner#123

Merged
long2ice merged 1 commit into
long2ice:mainfrom
mjpieters:method_signature
Apr 28, 2023
Merged

Attach updated endpoint signature to inner#123
long2ice merged 1 commit into
long2ice:mainfrom
mjpieters:method_signature

Conversation

@mjpieters
Copy link
Copy Markdown
Collaborator

@mjpieters mjpieters commented Apr 27, 2023

Not all endpoints accept a __signature__ attribute, nor should the cache decorator modify the decorated endpoint. Attach the signature to the returned inner function instead.

While here, refactor the signature updating code, and extract it to a separate function.

Fixes #115

Not all endpoints accept a __signature__ attribute, nor should the
cache decorator modify the decorated endpoint. Attach the signature
to the returned inner function instead.

While here, refactor the signature updating code, and extract it to
a separate function.
@mjpieters
Copy link
Copy Markdown
Collaborator Author

mjpieters commented Apr 27, 2023

Note: there is one edge-case the decorator isn't handling at the moment: if the decorated endpoint uses the names request or response but expects these to be a different type:

@app.get("/foo/{request}/{response}")
def foo(request: int, response: str) -> None:
    pass

This is perfectly valid, but the cache decorator will then fail with a ValueError: duplicate parameter name: 'request' or ValueError: duplicate parameter name: 'response' exception.

To make that work, you'd have to generate unique names for the request and response parameters. That's well outside the scope of this PR however.

@mjpieters
Copy link
Copy Markdown
Collaborator Author

Another issue is that the decorator assumes the request and response parameters always are named 'request' and 'response' when popping these from the copy_kwargs dict.

@long2ice long2ice merged commit 550ba76 into long2ice:main Apr 28, 2023
@long2ice
Copy link
Copy Markdown
Owner

Thanks!

@mjpieters mjpieters deleted the method_signature branch April 28, 2023 11:36
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 this pull request may close these issues.

AttributeError: 'method' object has no attribute '__signature__' in fastapi-cache2==0.2.0

2 participants