[PLAT-119] Add playlist indexing to DN #3073
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csjiang
commented
May 11, 2022
| @@ -299,22 +299,22 @@ class Track(Base): | |||
| genre = Column(String, nullable=True) | |||
| mood = Column(String, nullable=True) | |||
| credits_splits = Column(String, nullable=True) | |||
| remix_of = Column(postgresql.JSONB, nullable=True) | |||
There was a problem hiding this comment.
just standardizing references
csjiang
commented
May 11, 2022
| playlist_image_sizes_multihash = Column(String) | ||
| description = Column(String) | ||
| upc = Column(String) | ||
| metadata_multihash = Column(String, nullable=True) |
There was a problem hiding this comment.
making fieldz nullable so everything can go in metadat@
There was a problem hiding this comment.
are we missing an associated migration for these changes? otherwise models will mismatch actual dB state
hareeshnagaraj
suggested changes
May 11, 2022
| playlist_image_sizes_multihash = Column(String) | ||
| description = Column(String) | ||
| upc = Column(String) | ||
| metadata_multihash = Column(String, nullable=True) |
There was a problem hiding this comment.
are we missing an associated migration for these changes? otherwise models will mismatch actual dB state
|
|
||
|
|
||
| def create_entity(entity_type, instruction_data, transaction): | ||
| ENTITY_TYPE_TO_CREATE_MODEL_HANDLER = { |
There was a problem hiding this comment.
this is fine but can we add some explanation as to what is happening here? also there is definitely some missing stuff i think
There was a problem hiding this comment.
might be good to consolidate these and ENTITY_TYPE_TO_NAME in anchor_program_indexer. actually we could probably get away with just that and say if entity type is track call create_track_model etc since there's just a few types.
|
|
||
| def get_tablename(entity_type): | ||
| ENTITY_TYPE_TO_TABLENAME = { | ||
| entity_type.Playlist: "playlists", |
There was a problem hiding this comment.
instead of recreating this name can we reference models.__table__name or something? this adds another place to update tablename when we already have a definition
isaacsolo
reviewed
May 11, 2022
|
|
||
|
|
||
| # Custom error classes | ||
| class EntityExistenceException(Exception): |
There was a problem hiding this comment.
maybe a few comments about what these mean
| entity_exists = entity_id in db_models[tablename] | ||
| if entity_exists != entity_should_exist: | ||
| logger.info( | ||
| f"Skipping create {tablename} id {entity_id} because it exists: {entity_exists} when we expected: {entity_exists}." |
There was a problem hiding this comment.
maybe change tablename to entity_type
sliptype
pushed a commit
that referenced
this pull request
Sep 10, 2023
sliptype
pushed a commit
that referenced
this pull request
Sep 10, 2023
[e804aa1] [C-2248, C-2373] Use playlistUpdates, remove legacyNotifications (#3094) Dylan Jeffers [824933e] [C-2366] Improve web notification selection performance (#3103) Dylan Jeffers [4b8edef] [PLAT-696] Add trending-playlists/underground notifications (#3089) Dylan Jeffers [1f9cf3e] [C-2275] Fix android drawer offsets (#3095) Dylan Jeffers [fc14c82] [PAY-1063][PAY-1085][PAY-1086] Update UI for inaccessible gated tracks from favorites and history pages (#3100) Saliou Diallo [b0441f5] [C-2365] Update play buttons on web and mobile to show resume when track is current (#3101) Kyle Shanks [453910f] [C-2378] Add upload v2 feature flag (#3099) Sebastian Klingler [962a6df] [C-2337] Remove reachability mobile web (#3090) Raymond Jacobson [4ad5cd2] Fix visible collectibles for upload popup (#3093) Saliou Diallo [c143078] Fix feature flag bug (#3092) Saliou Diallo [44435b5] Fix upload prompt modal learn more url (#3091) Saliou Diallo [c9024ad] Use chat.messagesStatus instead of selector (#3087) Reed [38d43c4] [C-2369] Fix issue where notification poll can break app on signout (#3088) Dylan Jeffers [90122d9] [PAY-923] DMs: Add desktop entrypoints (#3083) Marcus Pasell [00f27e8] [PAY-907] Mobile chat reactions (#3020) Reed [4678b89] DMs: Fix broken typecheck on main (#3086) Marcus Pasell [756ade4] [PAY-1000][PAY-1084][PAY-1096][PAY-1097][PAY-1098] - More gated content fixes (#3085) Saliou Diallo [820aa9d] Fix upload and repost probers tests and lint (#3076) Sebastian Klingler [345607e] [C-2320] Fix profile socials alignment (#3079) Dylan Jeffers [569199c] Fix prod build timeout (#3084) Sebastian Klingler [12f6c22] Remove ports for local dev (#3082) Theo Ilie [1940618] Fix broken Main build due to typeerror (#3080) Marcus Pasell [eb8d47e] [PAY-1082] DMs: Dedupe sent messages (#3066) Marcus Pasell [50a11c3] Update SDK to 2.0.3-beta.0 (#3078) Marcus Pasell [c420fbb] Clean up NPM package lock (#3077) Marcus Pasell [35d1124] [C-2327] Add playlist updates slice (#3063) Dylan Jeffers [59862ad] [C-2344] Update the web playbar scrubber to respect the playback speed of podcasts (#3075) Kyle Shanks [ffeb0d3] [C-2349] Default download on wifi only to false (#3074) Andrew Mendelsohn [cafae41] [C-2325] Fix playlist table date-added column (#3073) Dylan Jeffers [384a510] [PAY-927] DMs: Empty messages state (#3068) Marcus Pasell [1132f83] Update @jup-ag/core to 2.0.0-beta.9 (#3072) Marcus Pasell [49c0ebf] [PAY-1072] Change "Download App" icon on Settings Page (#3067) Marcus Pasell [928dcaf] [PAY-1056] - More gated content updates and fixes (#3070) Saliou Diallo [1e1f769] [C-2345] Move PlaybackRate drawer to common drawers map (#3071) Kyle Shanks [f5d1251] Fix web-dist CI steps (#3069) Sebastian Klingler [5f89800] Fix heavy rotation playlist on client (#3056) sabrina-kiam [c0191e2] [C-2316] Add remote config for all oauth verification (#3052) Raymond Jacobson [40f5627] [PAY-1074][PAY-1075][PAY-1076][PAY-1080] - Update availability settings states + more QA fixes (#3059) Saliou Diallo [5be60ac] [C-2339] Update podcast control updates to also work for audiobooks (#3065) Kyle Shanks [163ebf5] [C-2297] Add fallback flag to podcast feature (#3064) Sebastian Klingler [f206391] [PAY-904] - Add gated content upload prompt (#3057) Saliou Diallo [1afc4e5] [C-1344] Move probers to monorepo and make tests pass (#3061) Sebastian Klingler [e198279] Remove random line (#3062) Saliou Diallo [24a001b] Add playback position logic for mobile (#3051) Kyle Shanks [d210124] [PAY-1070] Update TabSlider/SegmentedControl slider size on resize (#3044) Marcus Pasell
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds playlist indexing to DN.
Tests
Relying on integration tests + also testing locally manually.
How will this change be monitored? Are there sufficient logs?
Added a few new error types that come up in logs for metadata validation issues