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

refactor(backend): update rate caching #2891

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Conversation

mkurapov
Copy link
Contributor

@mkurapov mkurapov commented Aug 26, 2024

Changes proposed in this pull request

Delete the inflight request caching if we end up erroring. This is the inProgressRequests map in the rates service.

Context

fixes #2877

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels Aug 26, 2024
Copy link

netlify bot commented Aug 26, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 4072d0c
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/66cc21640be552000840a200

@mkurapov mkurapov marked this pull request as ready for review August 26, 2024 09:51
return rates
throw new Error(errorMessage)
} finally {
delete this.inProgressRequests[baseAssetCode]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BlairCurrey this is the line that fixes the error

Comment on lines -83 to -89
const isCloseToExpiry =
cachedExpiry.getTime() <
Date.now() + 0.5 * this.deps.exchangeRatesLifetime

if (isCloseToExpiry) {
this.fetchNewRatesAndCache(baseAssetCode) // don't await, just get new rates for later
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This didn't make sense, using this.fetchNewRatesAndCache(baseAssetCode) without await just turns it into a sync method, which was still making the request and not "prefetching it".

Now in the the new changes its a simple flow: if the rate is cached, return it, if not, get new rates. No need to check for "isCloseToExpiry"

@mkurapov mkurapov merged commit 5f4d0b5 into main Aug 26, 2024
42 checks passed
@mkurapov mkurapov deleted the 2877/mk/fix-rate-caching branch August 26, 2024 13:14
golobitch added a commit that referenced this pull request Sep 2, 2024
* refactor(backend): update rate caching (#2891)

* chore(localenv): fix startup migration (#2897)

* feat(backend): add trace to outgoing payment lifecycle (#2884)

* feat(backend): add trace to outgoing payment lifecycle

* feat(backend): encapsulate query lookup in trace

* chore(localenv): remove traces from default localenv

---------

Co-authored-by: Max Kurapov <[email protected]>

* Add migration files

* Update migration files

* Fix file name for migrations

* feat(tenant): basic tenant admin api schema and service

* Remove cascade deletion

* feat(auth): create basic tenant service and model plus graphql schema

* feat(graphql): generated data

* feat(graphql): add create tenant resolver and call service and update graphql schema

* feat(auth): add basic tenant schema and appropriate resources like model and service

* feat(generated): graphql schema

* feat(auth): add tenant id to create tenant input

* feat(auth): rename tenant id and add basic logic for calling create tenant service

* feat(auth): add basic create tenant functionality in service

* Add tenant model in backend

* feat(backend): add apollo client do dependencies

* feat(auth): add delete tenant mutation to the schema

* feat(auth): delete tenant

* Add tenantId field on resources models, update migration

* chore(auth): format

* feat(backend): create tenant service implementation

* feat(auth): codegen for putting generated files to backend

* feat(packages): make multi tenant work wip

* feat(mock-lib): add tenants to the seeding step

* feat(backend): update resolvers with tenant id and finish the tenant creation

* feat(localenv): update seed and docker compose

* feat(generated): graphql schema

* feat(bruno): admin auth create tenant mutation

* feat(backend): small changes to schema + mapping of tenant to graphql + bruno

* feat(everything): move endpoints to separate service, update bruno and schema do pagination and stuff

---------

Co-authored-by: Max Kurapov <[email protected]>
Co-authored-by: Nathan Lie <[email protected]>
Co-authored-by: bsanduc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: backend Changes in the backend package. type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not cache error rate requests
2 participants