Skip to content

AUD-859 Stop generating old routes on indexing flow #1789

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

Merged
rickyrombo merged 5 commits into from
Aug 24, 2021
Merged

AUD-859 Stop generating old routes on indexing flow #1789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ea8885
Stop generating old routes on indexing flow
rickyrombo Aug 18, 2021
a349a2b
remove extraneous session.commit()
rickyrombo Aug 18, 2021
301bdce
fix bad merge
rickyrombo Aug 20, 2021
1aecfab
Merge branch 'master' of https://github.com/AudiusProject/audius-prot…
rickyrombo Aug 24, 2021
fd8a948
Revert accidental removel of pending_track_routes
rickyrombo Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 2 additions & 66 deletions discovery-provider/src/tasks/tracks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, pending_track_routes):
"""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"]:
Expand Down Expand Up @@ -486,7 +424,6 @@ def parse_track_event(
track_metadata_multihash, track_metadata_format, creator_node_endpoint
)

add_old_style_route(session, track_record, track_metadata, pending_track_routes)
update_track_routes_table(
session, track_record, track_metadata, pending_track_routes
)
Expand Down Expand Up @@ -549,7 +486,6 @@ def parse_track_event(
upd_track_metadata_multihash, track_metadata_format, creator_node_endpoint
)

add_old_style_route(session, track_record, track_metadata, pending_track_routes)
update_track_routes_table(
session, track_record, track_metadata, pending_track_routes
)
Expand Down
38 changes: 14 additions & 24 deletions discovery-provider/tests/test_index_tracks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
[
Expand All @@ -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(
[
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 ===================

Expand Down