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

Mise à jour comment désactiver page cache #1878

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

AntoineAugusti
Copy link
Member

Depuis #1867, certains tests GBFS ne passent plus en particulier dans le controlleur smoove. Exemple

1) test Smoove GBFS conversion on station_status.json (GBFS.SmooveControllerTest)
   apps/gbfs/test/gbfs/controllers/smoove_controller_test.exs:30
   ** (RuntimeError) expected response with status 200, got: 502, with body:
   "{\"error\":\"smoove service unavailable\"}"

C'est étonnant car normalement il y a un mock qui est mis en place pour ce test spécifique. Je me suis donc demandé si PageCache ne pouvait pas avoir son rôle. Il semble que oui.

Actuellement, la logique pour désactiver le cache est de mettre un TTL à 0s.

@doc """
The purpose of this method is to help us set TTL to 0 during most tests, to disable cache for tests.
A better way (to be implemented in the future) will be to use behaviours and alternate implementations
(like explained in https://dashbit.co/blog/mocks-and-explicit-contracts), but that will do for now.
"""
def ttl_seconds(options) do
ttl_seconds = options |> Keyword.fetch!(:ttl_seconds)
disable_page_cache = Application.get_env(:gbfs, :disable_page_cache, false)
if disable_page_cache, do: 0, else: ttl_seconds
end

Ceci ne semble pas fonctionner

J'ai vérifié ça de 2 façons :

  • dans iex
Cachex.put(:my_cache, "key", "value", ttl: :timer.seconds(0)); Cachex.get(:my_cache, "key")
{:ok, "value"}
  • en mettant des IO.puts en cas de cache hit, et on voit ces logs apparaitre en exécutant les tests de Smoove

J'en déduis que mettre un TTL à 0s n'est pas suffisant. Le mécanisme de TTL est expliqué dans la doc en détails.

Cette PR propose donc d'utiliser Cachex.put tant que le cache n'est pas explicitement désactivé.

Copy link
Contributor

@fchabouis fchabouis left a comment

Choose a reason for hiding this comment

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

Pas de remarque.
Merci !

@AntoineAugusti AntoineAugusti merged commit e9931b9 into master Oct 26, 2021
@AntoineAugusti AntoineAugusti deleted the page-cache-disable branch October 26, 2021 08:00
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.

2 participants