Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

Fix eager discovery loader#1737

Merged
raymondjacobson merged 1 commit into
mainfrom
rj-fix-eager-loader
Aug 18, 2022
Merged

Fix eager discovery loader#1737
raymondjacobson merged 1 commit into
mainfrom
rj-fix-eager-loader

Conversation

@raymondjacobson

@raymondjacobson raymondjacobson commented Aug 18, 2022

Copy link
Copy Markdown
Member

Description

Eager loader is returning early if no cached value. Instead we should pull from the env vars set in the .env file.

Observed behavior is when you load localhost:3001/df/playlist/probers_playlist_do_not_delete-511 in incognito for the first time, it would crash b/c we can't initialize API client w/o some discovery node.

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

Locally w/ probers

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sweet so basically try cached, if not get from env?

@raymondjacobson

Copy link
Copy Markdown
Member Author

sweet so basically try cached, if not get from env?

Yep!

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/rj-fix-eager-loader

@raymondjacobson raymondjacobson merged commit 671cac1 into main Aug 18, 2022
@raymondjacobson raymondjacobson deleted the rj-fix-eager-loader branch August 18, 2022 03:01
@sliptype sliptype mentioned this pull request Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants