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

Update listen additional_info metadata added to spotify listens #1832

Merged
merged 11 commits into from
Sep 10, 2022

Conversation

alastair
Copy link
Collaborator

@alastair alastair commented Jan 28, 2022

We updated the documentation for what metadata to put in a listen. This PR updates listens added by our spotify reader to follow this format.

We added the following new fields to the documentation, but I didn't add all of them:

  • origin_url - added, same as spotify_id
  • music_service - added, 'spotify.com' as per our guides
  • submission_client - added, set to 'listenbrainz'

Not added:

  • media_player, media_player_version - we don't know what client the user is using to listen, so don't fill this in. It's not always going to be the spotify app
  • submission_client_version - does it make sense to have a "version" of the spotify reader?
  • music_service_name - we proposed music_service (as a domain), and music_service_name only if you are unable to identify a domain, so I skipped this. We could add it in anyway if we wanted, but I don't think it's needed.

Deprecated fields:

  • 'listening_from': 'spotify' because it's replaced by 'music_service': 'spotify.com'

@alastair
Copy link
Collaborator Author

Additional question - should we also add submission_client to the last.fm importer? I don't think we have data from this API to fill in any other fields.

@MonkeyDo MonkeyDo self-requested a review February 11, 2022 19:25
Copy link
Member

@amCap1712 amCap1712 left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@amCap1712
Copy link
Member

Adding the fields to Last.FM importer sounds good.

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

🚀

MonkeyDo and others added 4 commits June 29, 2022 17:38
to use
- additional_info.music_service (should be "spotify.com")
- additional_info.origin_url

Also removes a duplicate isListenFromThisService method which looks like an error and should be unused.
MonkeyDo and others added 4 commits July 4, 2022 18:31
Since this is used for testing if the listen is from spotify, this utility should test (with a regexp) if it is a spotify URL
@amCap1712 amCap1712 merged commit c3ed77f into master Sep 10, 2022
@amCap1712 amCap1712 deleted the spotify-read-metadata branch September 10, 2022 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants