From 7ea888536d3ddc02f1a24a534372cd731f795bb9 Mon Sep 17 00:00:00 2001 From: Marcus Pasell Date: Wed, 18 Aug 2021 23:28:20 +0000 Subject: [PATCH 1/4] Stop generating old routes on indexing flow --- discovery-provider/src/tasks/tracks.py | 76 +++---------------- discovery-provider/tests/test_index_tracks.py | 40 ++++------ 2 files changed, 27 insertions(+), 89 deletions(-) diff --git a/discovery-provider/src/tasks/tracks.py b/discovery-provider/src/tasks/tracks.py index 46dd7051873..492235afc15 100644 --- a/discovery-provider/src/tasks/tracks.py +++ b/discovery-provider/src/tasks/tracks.py @@ -213,70 +213,8 @@ def wrapper(*args, **kargs): @time_method -def add_old_style_route(session, track_record, track_metadata, pending_track_routes): - """Temporary method to add the old style routes to the track_routes db while we - transition the clients to use the new routing API. - """ - # Check if the title is staying the same, and if so, return early - if track_record.title == track_metadata["title"]: - return - # Use create_track_route_id() since the regex replace is slightly different - new_track_slug_title = helpers.create_track_route_id(track_metadata["title"], "") - # Remove "/" - new_track_slug_title = new_track_slug_title[1:] - # Append ID - new_track_slug_title = f"{new_track_slug_title}-{track_record.track_id}" - - # Check to make sure the route doesn't exist - existing_track_route = next( - ( - route - for route in pending_track_routes - if route.slug == new_track_slug_title - and route.owner_id == track_record.owner_id - ), - None, - ) - if existing_track_route is None: - existing_track_route = ( - session.query(TrackRoute) - .filter( - TrackRoute.slug == new_track_slug_title, - TrackRoute.owner_id == track_record.owner_id, - ) - .one_or_none() - ) - - if existing_track_route is None: - # Add the new track route - new_track_route = TrackRoute() - new_track_route.slug = new_track_slug_title - new_track_route.title_slug = new_track_slug_title - new_track_route.collision_id = 0 - new_track_route.owner_id = track_record.owner_id - new_track_route.track_id = track_record.track_id - new_track_route.is_current = ( - False # This route is meant to be a fallback, not the main route - ) - new_track_route.blockhash = track_record.blockhash - new_track_route.blocknumber = track_record.blocknumber - new_track_route.txhash = track_record.txhash - session.add(new_track_route) - - # Add to in-memory store to make sure we don't try to add it twice - pending_track_routes.append(new_track_route) - else: - logger.error( - f"Cannot add 'old-style' track_route to Track={track_record}\ - as the route already exists: {existing_track_route}" - ) - - -@time_method -def update_track_routes_table( - session, track_record, track_metadata, pending_track_routes -): - """Creates the route for the given track and commits it to the track_routes table""" +def update_track_routes_table(session, track_record, track_metadata): + """Creates the route for the given track""" # Check if the title is staying the same, and if so, return early if track_record.title == track_metadata["title"]: @@ -486,10 +424,15 @@ def parse_track_event( track_metadata_multihash, track_metadata_format, creator_node_endpoint ) +<<<<<<< HEAD add_old_style_route(session, track_record, track_metadata, pending_track_routes) update_track_routes_table( session, track_record, track_metadata, pending_track_routes ) +======= + # Note: This will flush the session + update_track_routes_table(session, track_record, track_metadata) +>>>>>>> 43ca3bce... Stop generating old routes on indexing flow track_record = populate_track_record_metadata( track_record, track_metadata, handle ) @@ -549,10 +492,15 @@ def parse_track_event( upd_track_metadata_multihash, track_metadata_format, creator_node_endpoint ) +<<<<<<< HEAD add_old_style_route(session, track_record, track_metadata, pending_track_routes) update_track_routes_table( session, track_record, track_metadata, pending_track_routes ) +======= + # Note: This will flush the session + update_track_routes_table(session, track_record, track_metadata) +>>>>>>> 43ca3bce... Stop generating old routes on indexing flow track_record = populate_track_record_metadata( track_record, track_metadata, handle ) diff --git a/discovery-provider/tests/test_index_tracks.py b/discovery-provider/tests/test_index_tracks.py index df20550914b..7820bf376e1 100644 --- a/discovery-provider/tests/test_index_tracks.py +++ b/discovery-provider/tests/test_index_tracks.py @@ -328,7 +328,7 @@ def test_index_tracks(mock_index_task, app): # Check that track routes are updated appropriately track_routes = session.query(TrackRoute).filter(TrackRoute.track_id == 1).all() # Should have the two routes created on track creation as well as two more for the update - assert len(track_routes) == 4, "Has four total routes after a track name update" + assert len(track_routes) == 2, "Has two total routes after a track name update" assert ( len( [ @@ -341,19 +341,8 @@ def test_index_tracks(mock_index_task, app): == 1 ), "The current route is 'real-magic-bassy-flip-2'" assert ( - len([route for route in track_routes if route.is_current is False]) == 3 - ), "Three routes are marked non-current" - assert ( - len( - [ - route - for route in track_routes - if route.slug == "real-magic-bassy-flip-2-1" - or route.slug == "real-magic-bassy-flip-1" - ] - ) - == 2 - ), "Has both of the 'old-style' routes" + len([route for route in track_routes if route.is_current is False]) == 1 + ), "One route is marked non-current" assert ( len( [ @@ -369,9 +358,9 @@ def test_index_tracks(mock_index_task, app): # ============== Test Track Route Collisions =================== # Attempts to insert a new track with the route "real-magic-bassy-flip" - # Should attempt to try to route to "real-magic-bassy-flip-1", but + # Should attempt to try to route to "real-magic-bassy-flip", but # that should be taken by the rename above, so the fallback logic - # should trigger making it go to "real-magic-bassy-flip-2" + # should trigger making it go to "real-magic-bassy-flip-1" event_type, entry = get_new_track_event_dupe() track_record_dupe = lookup_track_record( update_task, @@ -400,14 +389,15 @@ def test_index_tracks(mock_index_task, app): assert [ route for route in track_routes - if route.slug == "real-magic-bassy-flip-3" - and route.collision_id == 3 + if route.slug == "real-magic-bassy-flip-1" + and route.collision_id == 1 and route.is_current is True and route.title_slug == "real-magic-bassy-flip" - ], "New route should be current and go to collision id 3" + ], "New route should be current and go to collision id 1" - # Another "real-magic-bassy-flip", which should find collision id 2 and - # easily jump to collision id 3 and not need fallback logic + # Another "real-magic-bassy-flip", which should find collision id 1 and + # easily jump to collision id 2, but then conflict with the rename above + # and require the additional failsafe collision detection to go to collision id 3 event_type, entry = get_new_track_event_dupe() track_record_dupe = lookup_track_record( @@ -431,7 +421,7 @@ def test_index_tracks(mock_index_task, app): block_timestamp, pending_track_routes, ) - + session.commit() # Check that track routes are assigned appropriately track_routes = session.query(TrackRoute).filter(TrackRoute.track_id == 30).all() assert [ @@ -439,9 +429,9 @@ def test_index_tracks(mock_index_task, app): for route in track_routes if route.is_current is True and route.title_slug == "real-magic-bassy-flip" - and route.slug == "real-magic-bassy-flip-4" - and route.collision_id == 4 - ], "New route should be current and go to collision id 4" + and route.slug == "real-magic-bassy-flip-3" + and route.collision_id == 3 + ], "New route should be current and go to collision id 3" # ================== Test Revert TrackRoute =================== From a349a2b93307ebff8d755022026fe6c516a2eb8c Mon Sep 17 00:00:00 2001 From: Marcus Pasell Date: Wed, 18 Aug 2021 23:33:47 +0000 Subject: [PATCH 2/4] remove extraneous session.commit() --- discovery-provider/tests/test_index_tracks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discovery-provider/tests/test_index_tracks.py b/discovery-provider/tests/test_index_tracks.py index 7820bf376e1..d0398b7d011 100644 --- a/discovery-provider/tests/test_index_tracks.py +++ b/discovery-provider/tests/test_index_tracks.py @@ -421,7 +421,7 @@ def test_index_tracks(mock_index_task, app): block_timestamp, pending_track_routes, ) - session.commit() + # Check that track routes are assigned appropriately track_routes = session.query(TrackRoute).filter(TrackRoute.track_id == 30).all() assert [ From 301bdce4eec529a37d93c4a724625ac8d3e7a0de Mon Sep 17 00:00:00 2001 From: Marcus Pasell Date: Fri, 20 Aug 2021 16:25:33 +0000 Subject: [PATCH 3/4] fix bad merge --- discovery-provider/src/tasks/tracks.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/discovery-provider/src/tasks/tracks.py b/discovery-provider/src/tasks/tracks.py index 492235afc15..84a01b01110 100644 --- a/discovery-provider/src/tasks/tracks.py +++ b/discovery-provider/src/tasks/tracks.py @@ -424,15 +424,9 @@ def parse_track_event( track_metadata_multihash, track_metadata_format, creator_node_endpoint ) -<<<<<<< HEAD - add_old_style_route(session, track_record, track_metadata, pending_track_routes) update_track_routes_table( session, track_record, track_metadata, pending_track_routes ) -======= - # Note: This will flush the session - update_track_routes_table(session, track_record, track_metadata) ->>>>>>> 43ca3bce... Stop generating old routes on indexing flow track_record = populate_track_record_metadata( track_record, track_metadata, handle ) @@ -492,15 +486,9 @@ def parse_track_event( upd_track_metadata_multihash, track_metadata_format, creator_node_endpoint ) -<<<<<<< HEAD - add_old_style_route(session, track_record, track_metadata, pending_track_routes) update_track_routes_table( session, track_record, track_metadata, pending_track_routes ) -======= - # Note: This will flush the session - update_track_routes_table(session, track_record, track_metadata) ->>>>>>> 43ca3bce... Stop generating old routes on indexing flow track_record = populate_track_record_metadata( track_record, track_metadata, handle ) From fd8a948f5d49bcb0ccea387701ff659e19a671d4 Mon Sep 17 00:00:00 2001 From: Marcus Pasell Date: Tue, 24 Aug 2021 01:51:05 +0000 Subject: [PATCH 4/4] Revert accidental removel of pending_track_routes --- discovery-provider/src/tasks/tracks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discovery-provider/src/tasks/tracks.py b/discovery-provider/src/tasks/tracks.py index 84a01b01110..9ee2e1e9c70 100644 --- a/discovery-provider/src/tasks/tracks.py +++ b/discovery-provider/src/tasks/tracks.py @@ -213,7 +213,7 @@ def wrapper(*args, **kargs): @time_method -def update_track_routes_table(session, track_record, track_metadata): +def update_track_routes_table(session, track_record, track_metadata, pending_track_routes): """Creates the route for the given track""" # Check if the title is staying the same, and if so, return early