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

Nullable Episode.audio_preview_url #305

Merged
merged 3 commits into from Oct 31, 2023
Merged

Nullable Episode.audio_preview_url #305

merged 3 commits into from Oct 31, 2023

Conversation

Kuwertzel
Copy link
Contributor

@Kuwertzel Kuwertzel commented Oct 30, 2023

pydantic throws a ValidationError when an episode with the audio_preview_url field set to none is returned by the Spotify API. That is because the Episode base class currently expects str instead of Optional[str] for this attribute.

The Spotify API documentation declares the field as a nullable string.

I successfully tested the change using the following minimal example.
I'm not sure whether or not it would be appropriate to add a check for this change, but I did not create one. It feels wrong to use hard-coded episode IDs in a check since the episodes could change or be taken down in the future...

The existent checks seem to pass just fine, though i've never worked with tox before and might have overlooked something.

import tekore as tk

conf = ('<client_id>', '<client_secret>', 'http://localhost:8080/')
token = tk.prompt_for_user_token(*conf, scope=tk.scope.every)

spotify = tk.Spotify(token)

# works
print(f'with audio_preview_url: {spotify.episode("2VLjHtc7GdVcc2cokEbuI9").name}')

# does not work
print(f'without audio_preview_url: {spotify.episode("1pcBjfGIcXhSuoiBiPazHl").name}')

Related issue: I did not create one

  • Tests written with 100% coverage
  • Documentation and changelog entry written
  • All tox checks passed with an appropriately configured
    environment

PS: This is my first proper pull request, so let me know about anything I can improve :)

@felix-hilden
Copy link
Owner

Thanks! Especially for submitting the PR directly! The change looks good 👍 I'll try to release it this week along with the other open PR. The tests fail due to missing secrets.

@felix-hilden felix-hilden merged commit 1689194 into felix-hilden:master Oct 31, 2023
5 of 6 checks passed
@Kuwertzel Kuwertzel deleted the nullable-audio_preview_url branch November 7, 2023 20:51
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.

None yet

2 participants