Conversation
bruno-costanzo
force-pushed
the
feature/cache-authenticator
branch
from
b494701 to
80568f5
Compare
README.md
Outdated
| Ruber uses the `Ruber::Cache` class to store the access token. The cache is a simple in-memory cache that is cleared when the access token expires. | ||
|
|
||
| You can configure your own cache implementation by setting the `Ruber.cache` attribute. |
There was a problem hiding this comment.
Qué te parece decir que es para manejar la cache en lugar de el access token? El access token termina siendo un ejemplo de uso de la cache.
Además podríamos indicar cual es el uso por default.
| Ruber uses the `Ruber::Cache` class to store the access token. The cache is a simple in-memory cache that is cleared when the access token expires. | |
| You can configure your own cache implementation by setting the `Ruber.cache` attribute. | |
| Ruber uses a caching solution to improve efficiency (e.g., for caching tokens). By default, it uses [método por default], but you can change the cache method by setting the Ruber.cache attribute. |
There was a problem hiding this comment.
perfecto. Esto no lo pensé demasiado porque me pareció que lo importante era dejarlo escrito en algún lado y después revisar bien todo el readme en algún momento
| def read(key) = memory_store[key] | ||
| def write(key, value, _options = {}) = memory_store[key] = value | ||
| def clear = memory_store.clear | ||
| def delete(key) = memory_store.delete(key) | ||
| def memory_store = @memory_store ||= {} |
There was a problem hiding this comment.
Está bien que el método por defecto sea cachear en memoria? No conviene que sea un FileStore? Porque si se usa memoria se va a pedir el token en cada vuelta y Uber podría penalizarte o que superes la quota.
Entiendo que la idea es que los usuarios indiquen cual es su cache store de preferencia, pero me parece que estaría bueno que ya venga un store útil por defecto.
Qué decís?
There was a problem hiding this comment.
en realidad primero había hecho uno que devolvía todo nil y listo, pero para los tests esto es conveniente porque funciona como si tuvieras algún caché. No estoy seguro de qué convenga hacer por defecto.
lib/ruber/authenticator.rb
Outdated
|
|
||
| module Ruber | ||
| class Authenticator | ||
| OAUTH_URL = "https://auth.uber.com/oauth/v2/token" | ||
| CACHE_KEY = "uber_auth_token" |
There was a problem hiding this comment.
Este se usa? Creo que está inicializado pero luego se usa Ruber.cache_key.
Me pregunto si no conviene dejar la cache_key acá en lugar de tenerla en Configuration. No te parece mejor dejar en Configuration solo cosas que el usuario podría querer setear?
lib/ruber/authenticator.rb
Outdated
|
|
||
| class << self | ||
| def access_token | ||
| Ruber.cache.read(Ruber.cache_key) || fetch_new_token |
There was a problem hiding this comment.
No debería llamar a refresh_access_token si expiró?
There was a problem hiding this comment.
el refresh token lo pensé más como si por ejemplo en la caché tenés un token y cuando haces el llamado a uber te tira error o algo, entonces vas a hacer un refresh anticipado del token (por eso tmb hace un delete del cache key primero). No estoy seguro de qué conviene.
bruno-costanzo
force-pushed
the
feature/cache-authenticator
branch
from
d38e5bd to
3a815d2
Compare
| Ruber.cache.read(Ruber.cache_key) || fetch_new_token | ||
| @access_token = cached_token || fetch_new_token | ||
|
|
||
| @access_token = refresh_access_token if token_expired? |
There was a problem hiding this comment.
en qué contexto puede pasar que el token esté expirado acá?
se supone que cached_token guarda el token en caché por el tiempo de expiración que te devuelve la misma API, o no?
There was a problem hiding this comment.
ah ahí vi que también modificaste el write del caché para que el token quede guardado
2 participants
Authentication System with Cache
Why
What
access_tokenmethod to get cached token or fetch a new onerefresh_access_tokento force new token fetch