Fix/web api playlists v2 #3
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* pytest-capturelog plugin was merged into pytest core. * caplog.text is no longer callable. * pytest captures log messages of level WARNING or above, we need lower.
Updated tests for breaking changes in latest pytest.
This became an error in pytest v4.1.0. Also flake8 fix for now invalid escape sequence.
Workarounds for where we call fixures directly.
* docs: libspotify status update
yaourt is dead and not recommended.
When asserting mock.call in list(some_call), the some_call ends up on the lhs. In our failing test cases, the some_call on the lhs includes an ActorProxy and the mock.call on the rhs includes the mock.ANY, so we get ActorProxy.__eq__(mock.ANY), which returns False. In Pykka 1.x there was no ActorProxy.__eq__() defined so we ended up doing mock.ANY.__eq__(ActorProxy), which returned True.
- Simple dictionary-based caching for webclient GET requests with ETag support. - Respects max-age Cache-Control header. - Refresh token requests are not cached. - Query string normalised to ensure cache key is consistent. ETag response header is also stored in cache and subsequent requests set 'If-None-Match' header to the ETag value. Spotify service will respond (quickly) with 304 Not Modified status if data is unchanged.
Uses the default Travis build environment, which is now Xenial rather than Trusty (Python 2.7.12).
Cache playlist web API responses in a simple dict. playlists: Support Spotify's new playlist URI scheme (Fixes mopidy#215). search: uses 'from_token' market.
Spotify Track relinking guide says must use the original uri for any playlist manipulation etc so we should return that when translating to Mopidy models. libspotfy will handle any relinking when track is loaded for playback. Re-use track ref logic to extract correct URI. Added valid_web_data helper for checking common web object fields.
Block attemps to get user's playlists until first refresh completes. Temporarily increased timing logging verbosity. Using this as a baseline for optimisations where we: 1. Start Mopidy 2. Get the playlists (mpc lsplaylists) 3. Load a playlist of 454 songs (mpc load X). 1. ~7 seconds INFO 2019-09-12 23:00:21,985 Logged into Spotify Web API as kingosticks INFO 2019-09-12 23:00:22,346 playlists.as_list() took 360ms INFO 2019-09-12 23:00:28,830 Refreshed 66 playlists INFO 2019-09-12 23:00:28,830 Refresh Playlists took 6845ms INFO 2019-09-12 23:00:28,831 Starting Mopidy core 2. real 0m0.539s INFO 2019-09-12 23:01:03,461 playlists.as_list() took 270ms INFO 2019-09-12 23:01:03,641 playlists.as_list() took 176ms 2. real 0m0.202s INFO 2019-09-12 23:01:16,267 playlists.as_list() took 174ms 3. real 0m8.926s 3. real 0m0.410s 1. Cold cache. 2. Cache expired. 3. because libspotify has to load the all the track data.
Even once we have all playlist data from the Web API, any *track* lookups are done through libspotify which do a full load of the data. Clients may try to lookup all tracks in a playlist and this is therefore now very slow for large playlists. Previously, libspotify would start loading playlist track data in the background on first access to the playlist_container; this meant that any subsequent track lookups we did were fast. libspotify will do this background loading for any object once it has a Link to it so we can recreate this optimisation by simply grabbing libspotfy Link objects for all playlist track URIs we got from the Web API. We do need to maintain a reference to all these libspotify Links else they'll be freed and the track data will not be loaded. 1. ~7 seconds INFO 2019-09-12 23:07:28,755 Logged into Spotify Web API as kingosticks INFO 2019-09-12 23:07:35,521 Refreshed 66 playlists INFO 2019-09-12 23:07:35,521 Refresh Playlists took 6765ms INFO 2019-09-12 23:07:35,521 Starting Mopidy core 2. real 0m0.356s INFO 2019-09-12 23:07:55,680 playlists.as_list() took 143ms INFO 2019-09-12 23:07:55,830 playlists.as_list() took 147ms 2. real 0m0.167s INFO 2019-09-12 23:08:06,595 playlists.as_list() took 145ms 3. real 0m0.467s 3. real 0m0.399s 1. Unchanged 2. Unchanged 3. Big speedup for first call
Workaround for Spotify's use of max-age=0 for the me/playlists endpoint. 1. ~7 seconds INFO 2019-09-12 23:09:16,752 Logged into Spotify Web API as kingosticks INFO 2019-09-12 23:09:23,377 Refreshed 66 playlists INFO 2019-09-12 23:09:23,377 Refresh Playlists took 6625ms INFO 2019-09-12 23:09:23,377 Starting Mopidy core 2. real 0m0.194s INFO 2019-09-12 23:09:52,968 playlists.as_list() took 145ms INFO 2019-09-12 23:09:52,974 playlists.as_list() took 4ms 2. real 0m0.033s INFO 2019-09-12 23:09:54,108 playlists.as_list() took 13ms 3. real 0m0.353s 3. real 0m0.119s 1. Unchanged 2. Speedup for subsequent call 3. As above
kingosticks
force-pushed
the
fix/web-api-playlists-v2
branch
from
62a16f0 to
04154a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.