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

[TECH] Suppression de la mise en cache sur certaines routes pour gérer le cache par Baleen #11189

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

Mefl
Copy link
Contributor

@Mefl Mefl commented Jan 22, 2025

🥞 Problème

Certaines routes sont quasiment statiques mais sont servies à chaque fois car baleen ne les cache pas à cause du header cache-control

🥓 Proposition

supprimer le header cache-control

Remarque

Depuis Hapi.dev v17, config a été déprécié et remplacé par options. Toutefois le nom config restait pour compatibilité ascendante.
Or lors de nos tests, il semble que config.cache ne soit pas pris en compte, d'où le changement vers la notation state of the art afin de pouvoir changer le comportement du cache.

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Mefl Mefl force-pushed the tech-add-cache-header-on-route branch from 57a5f56 to d1f121b Compare January 22, 2025 13:59
@yaf yaf added Tech Review OK cross-team Toutes les équipes de dev team-captains This is your captain speaking labels Jan 27, 2025
@Mefl Mefl force-pushed the tech-add-cache-header-on-route branch from d1f121b to d0f0ce9 Compare January 27, 2025 14:29
@Mefl Mefl requested a review from a team as a code owner January 27, 2025 14:29
@lego-technix lego-technix changed the title [Tech] change cache header on route [TECH] change cache header on route Jan 27, 2025
@lego-technix lego-technix changed the title [TECH] change cache header on route [TECH] Suppression de la mise en cache sur certaines routes pour gérer le cache par Baleen Jan 27, 2025
@Mefl Mefl force-pushed the tech-add-cache-header-on-route branch from d0f0ce9 to 8c8e68a Compare January 27, 2025 15:20
Copy link
Contributor

@lego-technix lego-technix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bizarre, lorsque je teste en local le header cache-control est bien différent d'avant cette PR, mais il est toujours présent 🤔

Avant :

curl --insecure --include https://app.dev.pix.fr/api/feature-toggles
HTTP/1.1 200 OK
Server: nginx
Date: Mon, 27 Jan 2025 15:49:32 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 782
Connection: keep-alive
X-Powered-By: Express
cache-control: max-age=30, must-revalidate
Vary: origin, Accept-Encoding
access-control-expose-headers: WWW-Authenticate,Server-Authorization

Avec cette PR :

curl --insecure --include https://app.dev.pix.fr/api/feature-toggles
HTTP/1.1 200 OK
Server: nginx
Date: Mon, 27 Jan 2025 15:50:02 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 782
Connection: keep-alive
X-Powered-By: Express
Cache-Control: private, max-age=0, must-revalidate
Vary: origin, Accept-Encoding
access-control-expose-headers: WWW-Authenticate,Server-Authorization

Copy link
Contributor

@lego-technix lego-technix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Lu et testé fonctionnellement avec succès

Sur les RA, comme prévu, les routes modifiées n'ont bien pas de header cache-control.

Mefl added 3 commits January 27, 2025 16:08
…de Hapi.dev v17+

config étant gardé pour compatibilité ascendante mais ne permet pas de prendre en compte les nouveaux comportement comme la spéiification du comportement de header de cache.
@pix-service-auto-merge pix-service-auto-merge force-pushed the tech-add-cache-header-on-route branch from 8c8e68a to c412b26 Compare January 27, 2025 16:08
@pix-service-auto-merge pix-service-auto-merge merged commit 72de63d into dev Jan 27, 2025
9 of 10 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the tech-add-cache-header-on-route branch January 27, 2025 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team Toutes les équipes de dev 🚀 Ready to Merge team-captains This is your captain speaking Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants