[WIP] refactor: zerion api uses cache-first-datasource#2967
Conversation
Signed-off-by: dsh <11198975+LucieFaire@users.noreply.github.com>
|
@claude review |
| }) | ||
| .then(({ data }) => ZerionBalancesSchema.parse(data)); | ||
| await this.cacheService.hSet( | ||
| const data = await this.dataSource.get<ZerionBalances>({ |
There was a problem hiding this comment.
@PooyaRaki
The new path parses cache data as ZerionBalancesSchema ([{ data: [...] }], but the previous implementation wrote only the array ZerionBalanceSchema ([zerionBalances.data]) under the same zerion_balances_<address> cache key/field. So with the new implementation on cache hits it will deserialize to an array and throw ZodError until expiry, causing transient getBalances failures for already-cached wallets.
How critical it is? We can force clean cache for balances, or send a hook event to trigger it. Otherwise we need a fallback here to be able to work with both, but as cache is not long lived i wonder if we should invest in it. WDYT?
There was a problem hiding this comment.
As this endpoint is not used, the cache will be empty in any case.
LucieFaire
changed the title
LucieFaire
force-pushed
the
refactor/wa-1651-zerion-api-datasource-get
branch
from
da0b57e to
ab9313f
Compare
| ); | ||
| } | ||
|
|
||
| await this._checkRateLimit(); |
There was a problem hiding this comment.
I had to extract it above the get call, which now means the cache hits are also counted towards the RL. I can either do a cache pre-check here as it used to be (do double cache lookup), or i can extend datasource.get method to have a .onMiss callback function we can pass. The only thing its not a pattern anywhere else in code.
Not sure if I should just go back to how it was or implement additional stuff
cc @PooyaRaki
There was a problem hiding this comment.
If we have a rate limit check, I would check it properly.
Signed-off-by: dsh <11198975+LucieFaire@users.noreply.github.com>
LucieFaire
force-pushed
the
refactor/wa-1651-zerion-api-datasource-get
branch
from
ab9313f to
df0c902
Compare
vseehausen
left a comment
vseehausen
left a comment
There was a problem hiding this comment.
Approved with two comments.
Important: This endpoint is not used. We should rather remove it to clean up the codebase.
| ); | ||
| } | ||
|
|
||
| await this._checkRateLimit(); |
There was a problem hiding this comment.
If we have a rate limit check, I would check it properly.
| }) | ||
| .then(({ data }) => ZerionBalancesSchema.parse(data)); | ||
| await this.cacheService.hSet( | ||
| const data = await this.dataSource.get<ZerionBalances>({ |
There was a problem hiding this comment.
As this endpoint is not used, the cache will be empty in any case.
2 participants
Summary
Refactor network calls in Zerion API services to use
CacheFirstDataSource.get()instead of direct network calls with manual caching, aligning with the established pattern used throughout the codebase.WA-1651
Changes
ZerionWalletPortfolioApi.getPortfolio()refactored to usedataSource.get()ZerionBalancesApi.getBalances()refactored to usedataSource.get()ZerionBalancesApi.getCollectibles()refactored to usedataSource.get()ZerionBalancesApi._fetchChainMappingForNetwork()refactored to usedataSource.get()ZerionBalancesApiCacheFirstDataSource)