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

Fix primarySyncFromSecondary circular dep by moving initAudiusLibs to separate file #3506

Closed
wants to merge 2 commits into from
Closed

Fix primarySyncFromSecondary circular dep by moving initAudiusLibs to separate file #3506

wants to merge 2 commits into from

Conversation

SidSethi
Copy link
Contributor

@SidSethi SidSethi commented Jul 19, 2022 •
edited
Loading

Description

Problem
circular dependency between primarySyncFromSecondary and serviceRegistry: primarySyncFromSecondary imports serviceRegistry, which imports stateMachineManager, which eventually imports issueSyncRequest.jobProcessor which imports primarySyncFromSecondary

Solution

  • Move serviceRegistry._initAudiusLibs() to separate file
  • update serviceRegistry to consume above
  • update saveFileForMultihashToFS() and getCreatorNodeEndpoints() to accept libs instead of serviceRegistry
  • same changes to other files

Tests

Existing test coverage
TODO maddog regression test

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

Error log containing [primarySyncFromSecondary] and InitAudiusLibs Error
Logs containing circular dependency or initAudiusLibs

SidSethi
SidSethi commented Jul 19, 2022
View reviewed changes
creator-node/src/services/initAudiusLibs.js
* Creates, initializes, and returns an audiusLibs instance
*
* Configures dataWeb3 to be internal to libs, logged in with delegatePrivateKey in order to write chain TX
*/
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 function is moved from _initAudiusLibs() in serviceRegistry with following changes:

  • all config variables are defined at the top
  • thats it

SidSethi
SidSethi commented Jul 19, 2022
View reviewed changes
creator-node/src/services/sync/skippedCIDsRetryService.js
@@ -11,11 +11,9 @@ const LogPrefix = '[SkippedCIDsRetryQueue]'
* TODO - consider moving queue/jobs off main process. Will require re-factoring of job processing / dependencies
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only change in this file is to remove serviceRegistry import, and pass libs to saveFileForMultihashToFS`

SidSethi
SidSethi commented Jul 19, 2022
View reviewed changes
creator-node/src/fileManager.js
const { libs } = require('@audius/sdk')
const LibsUtils = libs.Utils
const { libs: audiusLibs } = require('@audius/sdk')
const LibsUtils = audiusLibs.Utils
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prevent name collision

@SidSethi SidSethi marked this pull request as ready for review July 19, 2022 20:01
@SidSethi SidSethi requested a review from dmanjunath July 19, 2022 20:01
@SidSethi SidSethi changed the title Fix primarySyncFromSecondary circular dep Fix primarySyncFromSecondary circular dep by moving initAudiusLibs to separate file Jul 19, 2022
@SidSethi SidSethi removed the request for review from dmanjunath July 19, 2022 20:03
@SidSethi
Copy link
Contributor Author

Closed in favor of #3507

@SidSethi SidSethi closed this Jul 19, 2022
@SidSethi SidSethi deleted the ss-fix-primarysyncfromsecondary-serviceregistry branch July 19, 2022 20:04
sliptype pushed a commit that referenced this pull request Sep 10, 2023
@dharit-tan
[PAY-1377] Compose message icon to TouchableOpacity (#3506)
eed5b90
sliptype pushed a commit that referenced this pull request Sep 10, 2023
v1.5.27
3098131 
[9a0592f] [PAY-1355] Add space before learn more link in inbox unavailable message (#3507) Reed
[989390c] [PAY-1267] Add space before placeholder in chat textinput (#3508) Reed
[a1e66ce] [PAY-1377] Compose message icon to TouchableOpacity (#3506) Reed
[dc12349] Remove write_metadata_through_chain flag (#3432) Michelle Brier
[5523041] Add clientVersion to mobile analytics (#3498) Sebastian Klingler
[e2ba8ed] [PAY-1343] First message in new chat shows purple unread indicator (#3503) Reed
[52096cf] Fix upload regression on stage (#3502) Andrew Mendelsohn
[cdf5663] [PAY-1175] DMs: Autofocus, clear search, elevate placeholder (#3486) Marcus Pasell
[d012240] [PAY-1318] Missing Shadow and Space Between Divider and Header (#3500) Michael Piazza
[e71845e] [PAY-1336] Updates to mobile chats user search screen (#3489) Reed
[9d12c62] [PLAT-1005] Parse for add track to playlist notification from backend (#3439) sabrina-kiam
[d9c62b1] [C-2708] Improvements around codepush & force pushing (#3501) Sebastian Klingler
[aa26688] [pay-1311] Reset chat state on sign out (#3499) Randy Schott
[6079466] [PAY-1337] Fix mobile chat message shadows (#3494) Reed
[643d3e0] [PAY-1305] DMs: Add skeletons for chats on web (#3496) Marcus Pasell
[5e5c900] [] Add styling fixes from mobile playlist updates QA (#3485) Kyle Shanks
[e182e8b] [PAY-1308] Use all messages for tracking latest received (#3484) Randy Schott
[1e16ea8] Push props down to SelectPage (#3495) Andrew Mendelsohn
[8993e52] [C-2705] Port upload files page to new upload form (#3493) Andrew Mendelsohn
[986b621] [PAY-1303] Add toastSagas to mobile (#3488) Marcus Pasell
[07c35e2] [PAY-1324][PAY-1326][PAY-1323][PAY-1327][PAY-1331][PAY-1328][PAY-1320] DMs UI fixes 2 (#3491) Michael Piazza
[f37c767] [C-2658] Fix favorite Hot & New in sign up (#3492) Sebastian Klingler
[404acd6] HOTFIX: left-nav delete-confirmation (#3490) Dylan Jeffers
[89ac0a5] [C-2699] Fix collection action button sizes (#3487) Dylan Jeffers
[bef1745] [PAY-1330][PAY-1317][PAY-1321] Polish DMs #1 (#3482) Michael Piazza
[c7611a8] Rename addToast to registerToast (#3472) Marcus Pasell
[614863c] [C-2700][C-2701] upload redesign feature flag and placeholder page (#3480) Andrew Mendelsohn
[1f70aa4] [PAY-1316] DMs: Do the thing (Fix mobile chats not populating) (#3483) Marcus Pasell
[4cf0c1e] [C-2702] Remove initialSelectedNode from disc-node selector (#3481) Dylan Jeffers
[6063b9e] SDK v2.0.3 beta.32 (#3479) Marcus Pasell
[c106ed8] [PAY-1283] DMs: Listen for chat messages app wide, and get unread message count on load (#3477) Marcus Pasell
[8845ae7] [PAY-1329] Fix marking chats as unread (#3478) Reed
[c6347af] Update end of message history copy on mobile (#3476) Reed
[2700165] [PAY-1293] Adjust portal and animations to get more consistent behavior on Android (#3475) Randy Schott
[afe7deb] [PAY-1295] DMs: Add a toast when chats fail to create (#3468) Marcus Pasell
[72d2788] DMs: Rename "Thread" to "Conversation" (#3474) Marcus Pasell
[5d69114] DMs: Add end of messages notice (#3466) Marcus Pasell
[e35a23f] Change delete thread copy to conversation on mobile (#3473) Reed
[31a6a55] [PAY-1301] DMs: Add empty state to search users modal (#3470) Marcus Pasell
[032107c] [PAY-1292] DMs: Delete chat style updates (#3469) Marcus Pasell
[7441321] [pay-1214] add patch to fix inverted list ANR bug (#3457) Randy Schott
[018ff3b] [PAY-1291] Adds new message toast when scrolled up on mobile (#3463) Randy Schott
[73b3ee0]  Add developer apps page, call new SDK developer apps + grants APIs [C-2624] (#3396) nicoback2
[09f08b3] [PAY-1294] Max length for chat text input on mobile (#3465) Reed
[ec305cb] [PAY-1278] Indicator for end of chat history on mobile (#3461) Reed
[593914c] Filter out self from create chat search results on mobile (#3462) Reed
[a42e359] Fix input with explicit input props (#3464) Dylan Jeffers
[5ada3f5] [PAY-1138] Fix headers on Chats and fix old header prod bug (#3442) Michael Piazza
[8966c0d] [C-2648] Add redesigned playlist management flow (#3456) Dylan Jeffers
[5fa7468] [PAY-1297] DMs: Don't show current user in search results (#3460) Marcus Pasell
[4a9824c] [C-2635] Fix offline playback; broken signature fetch (#3459) Andrew Mendelsohn
[d93537b] [PAY-1246] Mobile delete chat thread UI (#3458) Reed
[12107d4] [PAY-1288] DMs: Show max length in composer (#3454) Marcus Pasell
[9f8b112] [PAY-1276] Mobile chats change color on press (#3446) Reed
[bb53df1] [C-2646] Add idListArgKey for lists of entities; add fetchArgs types (#3452) Andrew Mendelsohn
[a954dce] [INF-415] Upgrade to react-native 0.71.8 (#3429) Sebastian Klingler
[1591fb2] Retry CodePush installs if they failed the first time C-2659 (#3451) nicoback2
[23f574b] Feature flag unread messages notif bubble (#3453) Reed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@SidSethi