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.

Sign up for GitHub

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

Jump to bottom

[CON-215] - Make bl init asynchronous but exit if err #3231

Merged
merged 9 commits into from
Jun 21, 2022
Merged

[CON-215] - Make bl init asynchronous but exit if err #3231

merged 9 commits into from
Jun 21, 2022

Conversation

vicky-g
Copy link
Contributor

@vicky-g vicky-g commented Jun 10, 2022 •
edited
Loading

Description

BL init was taking too long (3-5 min on prod). So, this PR:

  • removes the await, but still exits if the init fails. There will be a 3-5 minute gap on this service's functionality, but I think this is OK as the gap is pretty small.

Tests

  1. Forced throwing error on BL init
    await timeout(10000)
    throw new Error('I SHOULD BE HALTING APP START UP')
  1. Monitored CN logs and confirmed that app stopped working if init failed

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

The app will not work on startup and will print a BL error message

[2022-06-10T17:17:24.572Z] ERROR: audius_creator_node/289736 on 96edcfe157f1: ServiceRegistry ERROR || Could not initialize BL: I SHOULD BE HALTING APP START UP
[nodemon] app crashed - waiting for file changes before starting...

@vicky-g vicky-g requested a review from SidSethi June 10, 2022 17:21
@vicky-g vicky-g added the content-node Content Node (previously known as Creator Node) label Jun 10, 2022
SidSethi
SidSethi reviewed Jun 10, 2022
View reviewed changes
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

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

seems p straightfwd - don't think we need the separate initServicesAsynchronously step tho?

lets also hold off on merging until after code freeze, so we can soak on staging then foundation nodes for a sprint

creator-node/src/index.ts Show resolved Hide resolved
@vicky-g vicky-g changed the title Make bl init asynchronous but exit if err [CON-212] - Make bl init asynchronous but exit if err Jun 15, 2022
@vicky-g vicky-g changed the title [CON-212] - Make bl init asynchronous but exit if err [CON-215] - Make bl init asynchronous but exit if err Jun 15, 2022
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jun 15, 2022
@vicky-g vicky-g requested a review from dmanjunath June 15, 2022 19:48
@vicky-g vicky-g requested a review from SidSethi June 21, 2022 16:39
SidSethi
SidSethi approved these changes Jun 21, 2022
View reviewed changes
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

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

nice!

creator-node/src/index.ts Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 21, 2022
@vicky-g vicky-g requested a review from SidSethi June 21, 2022 22:44
@vicky-g vicky-g merged commit fad46b3 into master Jun 21, 2022
@vicky-g vicky-g deleted the vg-speed-up-bl-init branch June 21, 2022 23:31
raymondjacobson pushed a commit that referenced this pull request Jun 25, 2022
@vicky-g @raymondjacobson
[CON-215] - Make bl init asynchronous but exit if err (#3231)
cdfd16d 
* make bl init asynchronous but exit if err

* update init err msg

* remove comment

* add comment + simplify err msg

* revert non-relevant changes

* add time logs + thrown err update
raymondjacobson pushed a commit that referenced this pull request Jun 25, 2022
@vicky-g @raymondjacobson
[CON-215] - Make bl init asynchronous but exit if err (#3231)
c14c2f1 
* make bl init asynchronous but exit if err

* update init err msg

* remove comment

* add comment + simplify err msg

* revert non-relevant changes

* add time logs + thrown err update
sliptype pushed a commit that referenced this pull request Sep 10, 2023
@dylanjeffers
[C-2219] Hide desktop modals until needed (#3231)
bcd7278
sliptype pushed a commit that referenced this pull request Sep 10, 2023
v1.5.21
fa41720 
[ffbd520] [C-2511] Don't delay mutuals fetch (#3260) Andrew Mendelsohn
[c196de8] [C-2516] Always filter followed users from suggested (#3259) Andrew Mendelsohn
[5a4df3e] [C-2514] Fix empty related artists when logged out (web) (#3258) Andrew Mendelsohn
[9127806] [C-2484] Fix edit playlist actions (#3255) Kyle Shanks
[f668c58] [PAY-1162] Update rendering overflow for some "Supporting" lists (#3256) Randy Schott
[5dfe39b] fix lint (#3257) Alec Savvy
[b171916] [PAY-1166] Make SDK respect DN override (client-side) (#3246) Reed
[1a977b5] PLAT-725: fix browser push (#3252) Alec Savvy
[fa9be2d] Fix react-router issues (#3254) Dylan Jeffers
[7daa2a8] C-2502: extend store history (#3253) Alec Savvy
[b0379b1] [C-2501] Use proxy-wormhole libs (#3250) Dylan Jeffers
[b56a71a] Update TOS (#3251) Raymond Jacobson
[3142d23] Run CI flows when certain files change (#3248) Sebastian Klingler
[77805ca] Fix signout test (#3249) Sebastian Klingler
[aae6daf] [C-2493] separate state for suggested follows vs related artists (#3247) Andrew Mendelsohn
[115e084] [C-574] Add migration path to @react-spring/web (#3245) Dylan Jeffers
[0372252] Fix React errors about `key` property when rendering tables (#3244) Randy Schott
[3b03e2e] [C-2495] Refactor left-nav to typescript, update naming, improve perf (#3243) Dylan Jeffers
[21feb7e] [C-2486] Add Related Artists on Profile (mobile) (#3240) Andrew Mendelsohn
[e3180f6] PLAT-725: browser push misconfigured (#3242) Alec Savvy
[e90deb5] [C-2494] Refactor drag-drop to typescript + performance (#3237) Dylan Jeffers
[9d3cac4] Remove service selection modal (#3241) Theo Ilie
[587abd4] Fix call to player.getPosition (#3239) Andrew Mendelsohn
[6e85c32] Bump mobile version (#3238) Sebastian Klingler
[63fdba2] [PAY-937] Fix empty state for transactions table (#3234) Randy Schott
[7d81ff3] [C-2338][C-2487][C-2489] Add Related Artists on Profile (web) (#3230) Andrew Mendelsohn
[7a1b283] Fix dev-mode hot-key modals (#3236) Dylan Jeffers
[bb653f6] [C-2440] Remove useThemedStyles (#3232) Dylan Jeffers
[8ea7397] Fix lint in common (#3235) Saliou Diallo
[ba7f260] [PAY-1140] Prevent spacebar from attempting to play inaccessible tracks (#3227) Saliou Diallo
[b7ea4d9] Improve oauth errors/logging (#3233) Raymond Jacobson
[80d3609] [PAY-986][PAY-1087][PAY-1091] Fix a bunch of UI issues (#3211) Saliou Diallo
[8652e91] [C-2219] Hide desktop modals until needed (#3231) Dylan Jeffers
[c026885] [C-2429] Remove hls (#3225) Dylan Jeffers
[9a0687d] [C-2448] Fix common dependencies and lint (#3224) Dylan Jeffers
[fbabbbf] Revert "[PAY-827][PAY-1154] Use DN stream endpoint for track download and offline mode (#3202) (#3228) Saliou Diallo
[1500b7f] Only mark notifications as viewed when notif panel is open (#3229) Dylan Jeffers
[1c9df53] [PAY-1128] Update logic for showing "earn/claim $AUDIO" pill (#3218) Randy Schott
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SidSethi SidSethi SidSethi approved these changes

@dmanjunath dmanjunath Awaiting requested review from dmanjunath

Assignees

@theoilie theoilie

Labels
content-node Content Node (previously known as Creator Node) size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vicky-g @SidSethi @theoilie