Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Not caching requests except for GET #75

Open
jegork opened this issue Aug 11, 2022 · 9 comments
Open

Not caching requests except for GET #75

jegork opened this issue Aug 11, 2022 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@jegork
Copy link
Contributor

jegork commented Aug 11, 2022

Hello!

I do not completely understand why caching is disabled for methods except for GET, and if so, why is request passed as the first parameter to the function?

In my case, which is something in the lines of:

@cache
def redirect(request: Request):
    pass

this leads to an error, because parameter request is passed twice (once as request, and another in args/kwargs)

Thanks!

@jameswinegar
Copy link

Legitimate use case for caching POST would be ML model prediction. I think the reason it is only caching GETs is to limit surface area for testing. @long2ice any thoughts?

@jegork
Copy link
Contributor Author

jegork commented Aug 19, 2022

Thanks for the reply @jameswinegar, that is exactly why I was wondering, because I use the package to cache ML predictions.

@antont
Copy link

antont commented Sep 9, 2022

In my case for a normal CRUD like web api for a website I wouldn't want to cache POSTs, I guess the reasoning in the lib is similar.

You can make a modified decorator at least, that's what I'm doing for other reasons (made it work with SQLModel, and cache final JSONs, not py objects), but ofc caching POST might be nice and would be simple as an option in the lib too.

@aiwhoo
Copy link

aiwhoo commented Oct 27, 2022

Has this been examined more? We really need to cache post requests. How have people been working around it? @long2ice

@jegork
Copy link
Contributor Author

jegork commented Jan 21, 2023

A need to cache POST has been mentioned many times. Should I submit a PR to add it?
@long2ice mentioned that POST is not cacheable by definition, but the RFC here http://www.faqs.org/rfcs/rfc2616.html, section 9.5 states:

Responses to this method are not cacheable, unless the response
   includes appropriate Cache-Control or Expires header fields.

@long2ice
Copy link
Owner

OK, just do it

@mjpieters
Copy link
Collaborator

If #139 is accepted, the code base is going to be easier to update here. Just update the new _uncacheable() utility function and remove the if request.method != "GET": line.

Given that you can decorate an endpoint with @app.api_route("...", methods=["get", "put", "post"]) the @cache decorator should probably have a way of configuring what request methods to cache for, and default this to frozenset({"GET"}) (I prefer immutable structures here, but it should be flexible and accept any iterable of strings). Plus, a default set of request methods on the FastAPICache object as the global configuration.

@jameswinegar
Copy link

#186 PR that implements.

@admo1
Copy link

admo1 commented Feb 16, 2024

Any update ? It's still not available

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants