-
-
Notifications
You must be signed in to change notification settings - Fork 966
feat: add support for souin #2383
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
base: main
Are you sure you want to change the base?
feat: add support for souin #2383
Conversation
e1f2180
to
bd2b504
Compare
Note: after a discussion with souin maintainer (cf here), Surrogate Keys invalidation on postFlush has been resolved |
bd2b504
to
10ff02f
Compare
By the way, FYI the JWT authentication by Souin itself will be deprecated on the next minor release I'm working on. |
api/docker/caddy/Caddyfile
Outdated
cache * { | ||
|
||
} |
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.
The braces and *
matcher are not necessary.
cache * { | |
} | |
cache |
api/Dockerfile
Outdated
@@ -98,7 +98,8 @@ FROM caddy:2-builder-alpine AS app_caddy_builder | ||
|
||
RUN xcaddy build \ | ||
--with github.com/dunglas/mercure/caddy \ | ||
--with github.com/dunglas/vulcain/caddy | ||
--with github.com/dunglas/vulcain/caddy \ | ||
--with github.com/darkweak/souin/plugins/caddy |
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.
spaces -> tabs
--with github.com/darkweak/souin/plugins/caddy | |
--with github.com/darkweak/souin/plugins/caddy |
10ff02f
to
7a39d5c
Compare
7a39d5c
to
b28baf3
Compare
Ping @soyuka |
what about http-tests/cache-tests#112 ? If it works I'm 👍 on merging this, @dunglas ? |
That’s almost done, I have some points to discuss about the expectations. |
I think we can merge that while talking about the default behavior when no Cache-Control header given. They expect to not cache the response but the RFC doesn't give informations on that. (cc @mnot) |
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.
LGTM. Is it possible to simplify the Caddy config and just use the defaults provided by Souin?
The Caddyfile could just set the cache directive because by default it stores for 2 minutes. @JacquesDurand |
allowed_http_verbs GET POST | ||
api { | ||
souin | ||
} | ||
cdn { | ||
dynamic | ||
provider default | ||
} | ||
ttl 1000s | ||
timeout { | ||
backend 10s | ||
cache 100ms | ||
} |
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.
allowed_http_verbs GET POST | |
api { | |
souin | |
} | |
cdn { | |
dynamic | |
provider default | |
} | |
ttl 1000s | |
timeout { | |
backend 10s | |
cache 100ms | |
} |
Hi !
This is a first attempt to integrate Souin, a HTTP Cache handler that can be built as a module for Caddy. This is still a work in progress.
It contains a seemingly minimal caddy configuration, as well as some configuration for api-platform to handle cache and invalidation.
What works:
What I am not sure about:
With the current configuration, the
SouinPurger
is called onpostFlush
when an ApiResource is modified (PATCH request for instance), and the PURGE request is made to the souin API (https://localhost/souin-api/souin by default) with the correct headers.Here is an exemple of what caddy receives (and responds):
Nevertheless, requesting GET /greetings after said PATCH request still hits the cache
What still needs to be figured out: