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

Remove Network Monitoring Models #3422

Merged
merged 18 commits into from
Jul 11, 2022
Merged

Remove Network Monitoring Models #3422

merged 18 commits into from
Jul 11, 2022

Conversation

jonaylor89
Copy link
Contributor

Description

Remove the sequelize models from network monitoring because they aren't being used

The project uses raw sql for all queries

Tests

n/a since the models aren't be imported or used anywhere

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

n/a

@SidSethi
Copy link
Contributor

whats benefit of removing models? concern is that it makes it harder for someone to add these all back since they wouldn't know where to find the old models code

can you instead just add a models/README.md indicating these are not in use and may be outdated, and maybe just comment out the export lines

@jonaylor89
Copy link
Contributor Author

whats benefit of removing models? concern is that it makes it harder for someone to add these all back since they wouldn't know where to find the old models code

can you instead just add a models/README.md indicating these are not in use and may be outdated, and maybe just comment out the export lines

idk it seems much more confusing to keep them around. Especially since it seems we're going to keep moving forward with using raw SQL for adding new metrics.

Adding a README.md with a note saying they aren't used and might be outdated seems like it's asking for trouble

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.

sg

@jonaylor89 jonaylor89 merged commit ff81fbe into master Jul 11, 2022
@jonaylor89 jonaylor89 deleted the jn-rm-nm-models branch July 11, 2022 23:36
sliptype pushed a commit that referenced this pull request Sep 10, 2023
[2c08c63] [C-2653] Add shallow argument to audius-query hooks (#3450) Andrew Mendelsohn
[0e5d1b3] [PAY-1247] Cap height of chat textinput on mobile (#3447) Reed
[efaa425] [PAY-1243] Mobile chat header touchable feedback (#3448) Reed
[3980e00] [C-2647] Use entire cache to denormalize data (#3449) Andrew Mendelsohn
[1395822] [PAY-1150] Disable scrolling when chat reaction popup is active (#3445) Reed
[f4ad5f7] [C-2592] Fix edit playlist (#3443) Kyle Shanks
[875054a] Fix mobile native change comment C-2657 (#3444) nicoback2
[63b5ab4] Bump SDK version and change walletapi to auth (#3440) nicoback2
[1611465] DMs: Use unblock/block confirmation modals on profile (#3437) Marcus Pasell
[179dda6] [PAY-1142] DMs: Update chat tail (#3438) Marcus Pasell
[95f1843] [PAY-946] Handle signed out users across DMs (#3423) Michael Piazza
[682eebb] [C-2649] Refactor desktop collection header (#3441) Dylan Jeffers
[7310fae] [PAY-800][PAY-1249][PAY-1250] Display track and playlist tiles in web chats - Part 1 (#3377) Saliou Diallo
[689fbcb] [PAY-1274] DMs: Unsticky inbox unavailable message (#3435) Marcus Pasell
[0212e19] Fix unread count incorrect on mobile chats (#3430) Reed
[6743352] Update legacy playlist screens to use legacy inputs (#3436) Kyle Shanks
[fb25e74] [CON-678] v2 user profile edit (#3431) Theo Ilie
[abf827f] [PAY-1255] [PAY-1245] DMs: Add confirmation modals (#3424) Marcus Pasell
[536d3c6] Make mobile chat inbox unavailable message unsticky (#3434) Reed
[f11f266] [PAY-1273] Fix scrolling issues (#3433) Michael Piazza
[26ec389] [PAY-1258] Add toast when chat messages are copied (#3421) Randy Schott
[9ba1956] [PAY-1251] [PAY-1188] DMs: Add error message to failed message sends (#3428) Marcus Pasell
[14034ae] [C-2597][C-2651] Add audius-query usage docs (#3427) Andrew Mendelsohn
[16deebc] [C-2508] Update playback positions to be tracked per user id (#3374) Kyle Shanks
[2d6b278] [C-2644] Fix issue where new playlist drag-drop not persisting (#3422) Dylan Jeffers
[a9c2c5e] [PAY-1212] [PAY-1146] Reactions and message UI polish (#3414) Michael Piazza
[ab8bd6b] Type fix for hook options (#3426) Andrew Mendelsohn
[d167810] [PAY-1272] Make desktop DM text copyable (#3425) Michael Piazza
[7ba5973] [PAY-925] Adds unread message icons on web (#3418) Randy Schott
[645f0a5] [PAY-935] Fix ChatHeader issue (#3419) Michael Piazza
[cc1c6e8] Update mobile playlist create flows to conform with new designs (#3416) Kyle Shanks
[9701930] [PAY-1260] New chat tails on mobile (#3420) Reed
[dd2873e] [PAY-1241] Skeleton for mobile chat list screen (#3393) Reed
[a373e62] [PAY-1253] Mobile message resend button (#3413) Reed
[ddc5540] Fix type error track availability modal C-2632 (#3415) nicoback2
[19929b9] [C-2601 C-2627 C-2628 C-2638] Playlist library QA (#3411) Dylan Jeffers
[5a7f61a] [C-2468] Add desktop create flow (#3409) Dylan Jeffers
[b0c1583] Mark chat as read on ChatScreen entry (#3410) Reed
[6e120cd] [PAY-1222] Fix unread messages count (#3407) Michael Piazza
[95dd306] [C-2299] Fix NowPlayingDrawer navigation deduping bug (#3405) Sebastian Klingler
[01fcab4] [PAY-1195] Mobile chat screen unavailable states (#3402) Reed
[67b1788] [C-2598] Add types for audius-query hooks (#3406) Andrew Mendelsohn
[8bf784a] Increase space between chat reaction and copy message button (#3403) Reed
[2cba881] Fix upload collection C-2630 (#3404) nicoback2
[aee7653] [PAY-1242] Mobile chats copy message button (#3399) Reed
[01ec1fb] [PAY-1220] Fix chat receive bug (#3400) Michael Piazza
[0ad899c] Add useAuthenticatedCallback (#3392) Dylan Jeffers
[412ca92] Improve popup-menu a11y (#3394) Dylan Jeffers
[ecdfb7e] [PAY-1136][PAY-1129] Add infinite loading to albums on desktop / mobile web (#3371) Randy Schott
[35678eb] [PAY-1028] Fix mobile chat actions drawer touch feedback (#3388) Reed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants