mirror of
https://github.com/ramsayleung/rspotify.git
synced 2026-04-25 23:45:52 +03:00
[GH-ISSUE #218] Upgrading to v0.11: questions and support #72
Labels
No labels
Stale
bug
discussion
enhancement
good first issue
good first issue
help wanted
pull-request
question
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/rspotify#72
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @marioortizmanero on GitHub (Jun 19, 2021).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/218
Rspotify's v0.11 update introduces a lot of breaking changes. Thus, I think an issue like this might help with upgrading.
First, check out the upgrading guide in the changelog:
CHANGELOG.md. If you have any questions or need any help please let us know!@marioortizmanero commented on GitHub (Jun 19, 2021):
Do note that this version hasn't been released yet, but those using the
masterbranch might consider this useful in the meanwhile.@flip1995 commented on GitHub (Oct 5, 2021):
Is there an ETA when 0.11 will get released? What issues are blocking the release and can I help to address those?
@marioortizmanero commented on GitHub (Oct 5, 2021):
Hi @flip1995! The problem is that I've recently started Uni classes again and I haven't had time to push the release, but it's really really close. What's pending is:
maybe_asynccrate as indicated by https://github.com/fMeow/maybe-async-rs/issues/6. I already provided a base design in the latest comment, and I was waiting for opinions before implementing it, but the author is probably busy.The thing is that this issue is going to take a considerable amount of time to fix, and it'll single-handedly block the rspotify release at some point. I could just skip it and release the new version before fixing it, but (1) it will introduce breaking changes, and most importantly, (2) those who have rspotify twice in their dependency graph with async and sync will break. Not sure how important these two items are as opposed to releasing the new version finally, honestly.
If you can help on any of these go ahead (just let us know to avoid repeated work), and if you have any questions do ask them. The most fun one is probably fixing
maybe_asyncbecause writing proc macros is pretty cool imo.@flip1995 commented on GitHub (Oct 6, 2021):
Thanks for the summary! Sadly I'm also a bit low on time, so I can't make any promises. I may have some time to look into the async issue in about 2 weeks.
@ramsayleung commented on GitHub (Oct 13, 2021):
Since all PRs have been merged, I think, we are ready to release a new version crate or a pre-version crate now :)
PS: ooh, it reminds me that there is something left to be updated, some docs are not up-to-date, for example, the diagram of trait hierarchy. I should update them.
@marioortizmanero commented on GitHub (Oct 13, 2021):
I think once we merge your PR we're ready, as long as everyone is OK leaving #221 for a future version.
@marioortizmanero commented on GitHub (Oct 14, 2021):
Shall we? Do you want to do the honors, Ramsay?
@marioortizmanero commented on GitHub (Oct 14, 2021):
Three attempts later, we've finally released v0.11.2! The changes from v0.11.0 to v0.11.2 are just fixes for the builds in
docs.rs. https://docs.rs/rspotify/0.11.2/rspotify/Cheers!
@marioortizmanero commented on GitHub (Oct 14, 2021):
Also, here's my (super long) blog post about the new version: https://nullderef.com/blog/web-api-client/. It tells the whole story and showcases some of the cool features we've added. Thanks again to everyone who made this release possible ❤️
@aome510 commented on GitHub (Oct 14, 2021):
@marioortizmanero wow finally! Nice work, everyone who is involved 🎉
I have been waiting for
v0.11.0since I first startedspotify-player. Currently, I have to make a work around with the blocking APIs to be able to usetokio v1. I guess I can start a big migration now 😅Bookmarked the blog post! I haven't noticed you're the guy behind nullderef until now. Really enjoyed your previous blogs about Rust.
@hrkfdn commented on GitHub (Nov 6, 2021):
Hey there. Congratulations to your release and thanks for all the effort. I'm currently migrating ncspot to 0.11.x. and I'm struggling a little with the new
ItemPositionsstruct. I have a collection of playable IDs and playlist positions and would like to dynamically createItemPositionsbased on those as in the following example:The borrow checker returns the following error:
I'm a little bit at loss here. It works fine if I pass static literals, but I can't manage to fulfill the memory lifetime requirements for dynamic IDs/positions. Any ideas?
@marioortizmanero commented on GitHub (Nov 6, 2021):
ItemPositionscontains borrowed data, so you must construct their values beforehand into owned types. This will work:Not sure if it's the best way to do it, but try with something like that.
@hrkfdn commented on GitHub (Nov 7, 2021):
I just realized my example wasn't very good and the problem was actually in the creation of
PlayableIdtrait objects which I didn't include in the example 🤦 Your advice still applied, thanks a lot!@hrkfdn commented on GitHub (Dec 12, 2021):
Hey again, one more question. Previously I used to specify the limit and offset values to implement a "Show more results" button in ncspot (see screenshow below). I would like to use the iterable results, but I can't think of a way to stop iteration if the end of a page is reached. Do you have any ideas on how I could tackle this or plans to expose this information?
@marioortizmanero commented on GitHub (Dec 12, 2021):
Assuming you know the length of the page, you can use
.take(length), see https://doc.rust-lang.org/std/iter/struct.Take.html. Is that what you needed?@buzzneon commented on GitHub (Dec 27, 2021):
Good afternoon!
Really enjoying the changes that 0.11 brings, and I have almost my entire project converted (only took a few hours). Thanks for the hard work!
I'm stuck on one thing though: in 0.10 we had
user_playlist_trackswhich returned a page of tracks, and we could keep hitting that same method to get all the pages. I don't seem to see an equivalent in 0.11; there isuser_playlist, but that returns aFullPlaylistwith just a single page of tracks.Any advice?
Thanks!
@marioortizmanero commented on GitHub (Dec 27, 2021):
I'm very glad you've liked the changes so far. I was a bit nervous we'd get lynched at the beginning for having so many breaking changes haha.
user_playlist_trackssuffered many changes:playlist_tracksbecause the I think the endpoint itself changed (if you're already authorized you don't really need to indicate the user ID, so it makes sense).playlist_itemsbecause it actually works for episodes as well. You're probably looking for that: https://docs.rs/rspotify/latest/rspotify/clients/base/trait.BaseClient.html#method.playlist_itemsplaylist_items_manual: https://docs.rs/rspotify/latest/rspotify/clients/base/trait.BaseClient.html#method.playlist_items_manual@buzzneon commented on GitHub (Dec 27, 2021):
Thanks so much @marioortizmanero !
I can't believe it was right under my nose - I even went spelunking through the source and I couldn't find it 🤦♂️
Using
playlist_itemsworks like a charm! (Also, I 100% agree with it being renamed). Don't feel bad about making breaking changes, it's often necessary for progress .. sometimes a building needs to be gutted a bit before it can really be expanded upon.Wishing you the best for the New Years!
@buzzneon commented on GitHub (Dec 29, 2021):
I think I may have found a bug .. Querying for devices, sometimes I get the following error:
The device in question is a Roku 3 running the Spotify app. What's weird is that sometimes it does work .. so I'm not sure if, when it works, it returns
Tv, or one of the other types. Next time it works, I'll try and get the returned payload.The error is being generated by: convert_result.
Here's the full payload returned from the Spotify API:
@marioortizmanero commented on GitHub (Dec 29, 2021):
Unfortunately the device type is not very well documented: https://developer.spotify.com/documentation/web-api/reference/#/operations/get-a-users-available-devices. It doesn't even mention "TV" or "Tv". I would say this is a problem on Spotify's side, but we can surely fix it easily.
Case insensitive deserialization was brought up inserde(https://github.com/serde-rs/serde/issues/586), and it ended up being added toserde_aux: https://docs.rs/serde-aux/3.0.1/serde_aux/container_attributes/fn.deserialize_struct_case_insensitive.htmlIn order to not pull that entire dependency we can just copy-paste it into therspotify_modelcrate. I'll make a PR. Not sure how this can be reproduced but I guess it makes sense.Nvm, it's easier to just use
serde(alias)@buzzneon commented on GitHub (Dec 29, 2021):
Great news, thanks again @marioortizmanero ! 😄 👍
@buzzneon commented on GitHub (Dec 30, 2021):
For completeness - the fix in
masterworks like a charm, thanks again! 😄@marioortizmanero commented on GitHub (Dec 30, 2021):
Awesome, thanks for the bug report!
@buzzneon commented on GitHub (Feb 14, 2022):
Hi @marioortizmanero , it looks like a similar issue exists for the device type
avr:@ramsayleung commented on GitHub (Feb 15, 2022):
I have created a PR to fix this problem, you could retry after this PR merged :)
@buzzneon commented on GitHub (Feb 17, 2022):
That worked! Thanks so much!
Sorry for the delay in getting back to you, it was a busy weekend.
@sewnie commented on GitHub (Mar 13, 2022):
i feel like it is appropriate to post this here as the main library for handling API in spotify-tui is rspotify
@marioortizmanero commented on GitHub (Mar 13, 2022):
That is indeed an error in rspotify, thanks for reporting. I assume it's the
Typeenum inContext, inCurrentPlaybackContext. We could fix it by adding a (badly documented)collectionvariant. But would it be possible for you to share the full error, just to make sure?Edit: can confirm that tekore also has this variant:
github.com/felix-hilden/tekore@d1200964f5/tekore/_model/context.py (L5)@sewnie commented on GitHub (Mar 13, 2022):
it is quite long but sure.

i hope this doesn't have some dangerous information 👻
@marioortizmanero commented on GitHub (Mar 13, 2022):
You did well, the only sensitive thing may be what you censored. I can confirm that it's a problem in
Type, which should be fixed with #306. Thanks again for the help!@eladyn commented on GitHub (Apr 12, 2022):
Hi! I'm currently trying to port this code to the new
rspotifyversion and I'm having some problems finding a clean way to parse IDs / URIs.This method receives a spotify URI from user interaction and should start playback for the given URI. In the current version, it just uses
start_playbackand passes in a single context URI or a track URI depending on wether it containsspotify:track, in both cases just as aString.This is no longer that easy. Not only has Spotify added shows, from which one can play episodes (
spotify:episode, I assume), the tricky bit for me is the new way to parseIds. To improve usage, thestart_playbackmethod was additionally split up intostart_uris_playbackandstart_context_playback.Due to the new way,
Ids are represented in the crate, namely as different structs that implement common traits such asId,PlayContextIdorPlayableId, I need to somehow parse my string into one of the types and call the correct one of the two methods.I'm currently doing something similar to the following and I'm wondering, if and how this could be achieved in a better way:
Stringmore or less the same it is done hereTypeof the parsed Uri, decide I want to turn the id into aPlayContextIdor aPlayableId(or rather something that implements the trait)TrackorEpisode:Parse the Id with the proper method (
TrackId::from_id(...)orEpisodeId::from_id(...)) and put it into aBox(ugh, heap allocation 😃), so that I can treat those two different things asBox<dyn PlayableId>.Parse the id with the proper method (see above), put it into a
Boxand insert that into a customnewtypestruct, which takes aBox<dyn PlayContextId>. This struct implementsPlayContextIditself (although not completely, it would panic on_type_staticandfrom_id_unchecked).The reason I (believe to) have to do this, is that
start_context_playbackis generic over itscontext_uriparameter and requires it to implementPlayContextId. As such, I can't just use aBox<dyn PlayContextId>, since that does not implement the trait itself. This feels really hacky.Sorry, if this issue is not the right place to ask this question, feel free to move it elsewhere, if not. Some of the issues I encountered might not be "Upgrading to v0.11" specific since the last crate version that
spotifydused, was0.8. (😬)Some of my problems should be fixed with https://github.com/ramsayleung/rspotify/pull/305, although things like parsing an Id that I know nothing about I would still have to implement myself. If you'd like me to, I can add my thoughts on that over there.
Anyway, thanks for this great library and sorry for this massive wall of text! 😅
@marioortizmanero commented on GitHub (Apr 26, 2022):
Hi @eladyn, just wanted to let you know that I wasn't able to answer yet because I haven't had any free time (and it will continue that way until around the end of May, unfortunately).
I can agree with you that the design of
Idtypes is indeed not perfect. I put a lot of effort into considering the different alternatives we had, and chosedynbecause it seemed like the most "idiomatic" and "natural" way to approach the problem. However, it still had a few downsides:String). I think this could be "fixed" by usingCow<str>but I am currently not sure.Idtype from adynNote that you may not need to box your Id if you don't plan on keeping it after the function. You can use a
&dyninstead.I agree that the resulting code is indeed overly complex, but only because you have to implement your own
Idtype. Handling both cases ofstart_context_playbackandstart_uris_playbackis inherently cumbersome if you want to do it in a type-safe way. As you said, you know nothing about the Id at that point. Therefore, I would consider this issue fixed if we managed to remove the custom Id part.I'm glad you at least got it working, though. If it's fine by you, once either I or Ramsay have more time, we can look into the issue and fix it properly in a future version.
@eladyn commented on GitHub (Apr 28, 2022):
Thank you for the answer! I don't have much time myself currently, so no hurries. I might have a look at the code I wrote again some time in the future and hopefully find a way to improve it a bit (maybe apply your suggestion about the
Boxthing). However, good to know that I wasn't just too dumb and didn't overlook an obvious solution. 😀