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

Fix top playlists with user_id #4701

Merged
merged 2 commits into from
Feb 2, 2023
Merged

Fix top playlists with user_id #4701

merged 2 commits into from
Feb 2, 2023

Conversation

jowlee
Copy link
Contributor

@jowlee jowlee commented Feb 1, 2023

Description

Updates the discovery endpoint /v1/full/playlists/top to accept an encoded user_id
Update the get_top_playlists query to not break on a none filter arg
Update the get_top_playlists query to accept a user_id param in the arg - falling back to the legacy fetching of user_id from x-user-id as an interim backup to not break existing clients
Updates libs binding for this endpoint to allow the user_id param

Tests

Ran libs linked locally and checked that the request was made with the correct parameter

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

@@ -44,7 +50,7 @@ def get_top_playlists(kind: TopPlaylistKind, args: GetTopPlaylistsArgs):
limit = args.get("limit", 16)
mood = args.get("mood", None)

if "filter" in args:
if args.get("filter") is not None:
Copy link
Contributor Author

@jowlee jowlee Feb 1, 2023

Choose a reason for hiding this comment

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

@alecsavvy Note that this broke in #4655 after the filter was added as a url param - causing it to pass in as None if not specified in the url param which then errored on the subsequent lines 55-56

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i see the difference, thanks!

@jowlee jowlee merged commit 4016364 into main Feb 2, 2023
@jowlee jowlee deleted the jowlee-top-playlist-fix branch February 2, 2023 17:56
jowlee added a commit that referenced this pull request Feb 2, 2023
jowlee added a commit that referenced this pull request Feb 2, 2023
audius-infra pushed a commit that referenced this pull request Feb 4, 2023
## Changelog

- 2023-02-03 [5fa6731] Remove user bank from sdk signup (#4719) [Raymond Jacobson]
- 2023-02-03 [5b9edbb] NO-TICKET: fix link for template generation in SDK (#4718) [Alec Savvy]
- 2023-02-02 [4016364] Fix top playlists with user_id (#4701) [Joseph Lee]
- 2023-01-30 [af22669] PLAT-623: death to the jdk (#4682) [Alec Savvy]
- 2023-01-25 [2d36d63] SDK: Don't fail healthcheck on 0 blockdiff (#4670) [Marcus Pasell]
- 2023-01-25 [894048c] Bump sdk to v1.0.43 [audius-infra]
audius-infra pushed a commit that referenced this pull request Feb 4, 2023
## Changelog

- 2023-02-03 [5fa6731] Remove user bank from sdk signup (#4719) [Raymond Jacobson]
- 2023-02-03 [5b9edbb] NO-TICKET: fix link for template generation in SDK (#4718) [Alec Savvy]
- 2023-02-02 [4016364] Fix top playlists with user_id (#4701) [Joseph Lee]
- 2023-01-30 [af22669] PLAT-623: death to the jdk (#4682) [Alec Savvy]
- 2023-01-25 [2d36d63] SDK: Don't fail healthcheck on 0 blockdiff (#4670) [Marcus Pasell]
- 2023-01-25 [894048c] Bump sdk to v1.0.43 [audius-infra]
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.

None yet

3 participants