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

Is the cache-control header set? #156

Closed
ceteri opened this issue Sep 1, 2023 · 9 comments
Closed

Is the cache-control header set? #156

ceteri opened this issue Sep 1, 2023 · 9 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@ceteri
Copy link

ceteri commented Sep 1, 2023

The async support and client-side caching here look great.

However, from testing, it appears that use of the @cache decorator here does not set a cache-control header?

Is there any reason that was omitted?

@AIGeneratedUsername
Copy link

AIGeneratedUsername commented Sep 1, 2023

Could you please clarify your use case and what you want to achieve or share a small code example?

If you are talking about caching HTTP responses with a minimal setup on your side, then you can look at https://github.com/requests-cache/aiohttp-client-cache or https://github.com/requests-cache/requests-cache

@ceteri
Copy link
Author

ceteri commented Sep 1, 2023

When HTTP page response get cached, shouldn't there also be directives communicated to the CDN and client through Cache-Control headers:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

As far as I can tell, the @cache decorator here does not modify FastAPI/Starlette response headers?

For example, using @cache(ttl="30s") should that also set a directive such as

Cache-Control: max-age=30

Handling ETags are another matter.

@ceteri
Copy link
Author

ceteri commented Sep 1, 2023

Also, my apologies if I'm conflating the intended usage.

It has been a common practice for cache libraries on web frameworks to support both the caching strategy and the directives from the decorator. Although clearly that's not needed if the cache is used internally, not for HTTP routes.

@Krukov
Copy link
Owner

Krukov commented Sep 17, 2023

Yes, this library is framework agnostic.

However there are some experimental functionality that's comes in 6.3.0 version and not documented yet

A cashews.contrib.fastapi module with a set of middlewares: CacheEtagMiddleware, CacheRequestControlMiddleware, CacheDeleteMiddleware

CacheEtagMiddleware - will add the Etag + If-None-Match headers if cache was used during the request handle
CacheRequestControlMiddleware - will handle Cache-Control request and response header, properly disable cache on Cache-Control no-store/no-cache/max-age-0 and add Age and Cache-Control header if cache was used during the request handle
CacheDeleteMiddleware - handle Clear-Site-Data

@ceteri I will be happy if you will try to use it, but at your own risk because it not tested much yet ( but I think that is better then https://github.com/long2ice/fastapi-cache )))

from cashews.contrib.fastapi import CacheRequestControlMiddleware

app = FastAPI()
app.add_middleware(CacheRequestControlMiddleware)

@Krukov
Copy link
Owner

Krukov commented Oct 15, 2023

I'll keep this issue open till I will add documentation about this "contrib" module

@Krukov Krukov added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 15, 2023
@ceteri
Copy link
Author

ceteri commented Oct 15, 2023

Thank you @Krukov yes, that looks really good.

from cashews.contrib.fastapi import CacheDeleteMiddleware, CacheEtagMiddleware, CacheRequestControlMiddleware

Ultimately it might also be helpful to include features that take into account the edge (CDN) cache rules, and how the middleware needs to interact with those. For example, https://developers.cloudflare.com/cache/how-to/cache-rules/

We have not seen any caching libraries focus on the edge or intermediate storage grids used by edge caches. We've built out code to handle this, although it is not generalized in a way that it could be used readily in a framework-agnostic library.

@shijo-keyvalue
Copy link

shijo-keyvalue commented Nov 8, 2023

Tried adding middleware as mentioned in the above comments :

app.add_middleware(CacheRequestControlMiddleware)

But I am getting this error while starting the server.

...lib/python3.7/site-packages/cashews/contrib/fastapi.py", line 86, in CacheRequestControlMiddleware
    def _to_disable(cache_control_value: Optional[str]) -> tuple[Command]:
TypeError: 'type' object is not subscriptable

@ceteri @Krukov

@Krukov
Copy link
Owner

Krukov commented Nov 8, 2023

Hi @shijo-keyvalue - Looks like a bug with python3.7 support , I will take a look at weekends. Also I found a few bugs with CacheRequestControlMiddleware - I gonna fix it soon, stay tuned )

@Krukov
Copy link
Owner

Krukov commented Jan 2, 2024

Added a few words in README (documentation) about contrib module and integrations/tools for an fastapi app

@shijo-keyvalue Sorry but I dropped 3.7 support as we don't have CI pipelines for it and it's a very "unpopular" version.

@Krukov Krukov closed this as completed Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants