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

Use Web API for Playlists #188

Open
wants to merge 18 commits into
base: develop
from

Conversation

9 participants
@kingosticks
Copy link
Member

commented Jun 11, 2018

This branch is aimed at fixing #140 and #182. It's still a work in progress but any thoughts are very welcome.

Might even be worth splitting the WebSession part out first.

  • Add Etag support
  • Save cached data to file on exit and restore on login. - don't think we need this
  • Translator should use @memoized decorator, or some kind of expiry aware version?
  • Track translators need to consider market availability
  • Tests
  • Consider refresh behaviour
import requests

from mopidy_spotify import utils

logger = logging.getLogger(__name__)
INFO = logging.getLevelName('INFO')

This comment has been minimized.

Copy link
@jodal

jodal Jun 14, 2018

Member

This is already available as logging.INFO.

This comment has been minimized.

Copy link
@kingosticks

kingosticks Jun 19, 2018

Author Member

Thanks!

@kingosticks kingosticks force-pushed the kingosticks:fix/web_api_playlists branch 2 times, most recently from 05d3b40 to ccf83e5 Jun 19, 2018

@kingosticks kingosticks referenced this pull request Jun 25, 2018

Closed

WIP: Support for playlists via Web API #189

2 of 6 tasks complete

@kingosticks kingosticks changed the title OAuthClient now cache header aware. Use Web API for Playlists Jun 26, 2018

@earthican

This comment has been minimized.

Copy link

commented Jun 26, 2018

I recently have just come across the playlists issue, so thank you so much @kingosticks for working on this.

Do you have a timeline on when this will get merged and released?

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2018

Sorry, I don't have a timeline for you. I've got plenty on this week, I'll spend time when I can.

@earthican

This comment has been minimized.

Copy link

commented Jun 27, 2018

No worries, thanks for the update, and I appreciate your work on this!!

kingosticks added some commits Jun 7, 2018

OAuthClient now cache header aware.
WebSession class hides web api complexity.

Use Web API for fetching playlists
Load all user playlist data at startup and force it be cached for 30 …
…seconds.

WebCache class allows better caching control.

Display timing info whilst debugging.
Never cache refresh token responses.
OAuthClient now supports any dict-like cache object.
WebResponseCache is just a specialised dict.
Ignore unplayable tracks.
Consider returning a Track's linked_from uri instead.
Spotify's Track Relinking guide warns about which Track URI to use for playlist manipulation.
We would then have to translate the unplayable linked_from URI to the relinked one before playback.

Fixed missing field parameter when fetching a Playlist's first track page (offset=100).

 and translating to the playable uri before playback.
Return original URI for relinked tracks.
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.
Memoize web_to_* functions using uri and name for the key.
Performance improvement is ~3x.

The timings below show a couple of playlist.*() funcs doing a web request via get_playlist()
and then translating the result. The 2nd and 3rd calls to get_playlist() are quick since
cached data was still valid.

Before:
    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 202ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 10ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 11ms
    INFO     playlists.lookup(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 48ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 226ms
    INFO     playlist.get_items(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 236ms

After:
    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 257ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 9ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 11ms
    INFO     playlists.lookup(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 13ms

    INFO     get_playlist(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 213ms
    INFO     playlist.get_items(spotify:user:bob:playlist:FrAgBGjDROklxbsbGrtLzDw) took 215ms
Skip refreshing token on cache hit.
Since only GET requests care about caching, the extra complexity doesn't
need to be in the generic _request_with_retries() function.
Plus, handling it earlier in get() means we won't needlessly refresh the token.

WebResponse gets an is_valid property rather than an expired() function.
Added Web API ETag support.
Performance improvement noticable for large playlists that would otherwise
fetch many full Track page.

Fixed broken playlist error logging.

@kingosticks kingosticks force-pushed the kingosticks:fix/web_api_playlists branch from ac4af8f to dc28fa6 Jul 21, 2018

kingosticks added some commits Jul 21, 2018

Web session fetches username from profile.
Factored out playlist data load.

Reduced/removed logging.
Playlists: Improve performance of subsequent playlist track lookups.
Even once we have all playlist data from the Web API, future track lookups are done
through libspotify which must then load the data again from scratch. 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 enabled any subsequent track
lookups we did to be fast. Since libspotify will do this background loading for any
object once it has a Link to it, we can recreate this optimisation by simply grabbing
libspotfy track Links for all playlist track URIs once we get them from the Web API.
We do need to hold a reference to the Links else the Libspotify Link will be freed
and the data will not be loaded.

@kingosticks kingosticks force-pushed the kingosticks:fix/web_api_playlists branch from dc28fa6 to 1e26ecf Jul 21, 2018

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Jul 21, 2018

So I (force, sorry) pushed some more fixes and cleanups. It's performing nicely now, the last commit made a big difference to this (for certain clients). Functionally I think I'm done, just the small issue of tests remain...

@brownjd

This comment has been minimized.

Copy link

commented Aug 1, 2018

Tried the following using this branch:

core = self.frontend.core
core.tracklist.clear()
core.tracklist.add(uri=self.playlists[self.selected].uri)
core.playback.play()

where the url was of the form 'spotify:user:username:playlist:xxxxxxxxxx'

Here's the error:

ERROR
SpotifyBackend backend caused an exception.
Traceback (most recent call last):
File "/home/pi/.local/lib/python2.7/site-packages/mopidy/core/library.py", line 19, in _backend_error_handling
yield
File "/home/pi/.local/lib/python2.7/site-packages/mopidy/core/library.py", line 236, in lookup
result = future.get()
File "/home/pi/.local/lib/python2.7/site-packages/pykka/threading.py", line 52, in get
compat.reraise(*self._data['exc_info'])
File "/home/pi/.local/lib/python2.7/site-packages/pykka/compat.py", line 12, in reraise
exec('raise tp, value, tb')
File "/home/pi/.local/lib/python2.7/site-packages/pykka/actor.py", line 201, in _actor_loop
response = self._handle_receive(message)
File "/home/pi/.local/lib/python2.7/site-packages/pykka/actor.py", line 295, in _handle_receive
return callee(*message['args'], **message['kwargs'])
File "/usr/local/lib/python2.7/dist-packages/Mopidy_Spotify-3.1.0-py2.7.egg/mopidy_spotify/library.py", line 34, in lookup
uri)
File "/usr/local/lib/python2.7/dist-packages/Mopidy_Spotify-3.1.0-py2.7.egg/mopidy_spotify/lookup.py", line 34, in lookup
return _lookup_playlist(session, web_session, config, uri)
NameError: global name '_lookup_playlist' is not defined

Is there a better way to play a playlist?

@brownjd

This comment has been minimized.

Copy link

commented Aug 1, 2018

Figured it out:

def lookup_playlist(session, web_session, config, uri):
needs to be
def _lookup_playlist(session, web_session, config, uri):

line 94 in lookup.py

@brownjd

This comment has been minimized.

Copy link

commented Aug 2, 2018

This is working well. Any chance of supporting Spotify Radio? The last issue was closed a few years ago because the underlying library did not support it. It seems like you're working around that now...

@justincrosby

This comment has been minimized.

Copy link

commented Aug 14, 2018

@brownjd's fix to this branch works. Playlists loaded via mopidy/home-assistant are working again. Thanks @kingosticks

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2018

I've split the web client caching parts of this into #195 and added tests.

The playlist part will follow.

@kingosticks kingosticks referenced this pull request Aug 19, 2018

Open

Web client caching #195

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2018

@princemaxwell The old style i.e. spotify:user:spotify:playlist:37i9dQZF1DX0XUsuxWHRQd will work. But that form needs adding too. Thanks

EDIT: I lied, the old style was also broken! But the latest commit fixes that, at least.

@kingosticks kingosticks force-pushed the kingosticks:fix/web_api_playlists branch from ba1f963 to cad8c98 Oct 11, 2018

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 11, 2018

Sure, old links were broken too.

Do this commit really effect MPC????

Because i removed Mopidy-Spotify (only this package) for testing. And MPC was still working for artist, track and album URIs of Spotify...

fix broken playlist track lookups.
Also handle 'spotify:playlist:<spotifyID>' style links.
@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2018

I am afraid I don't understand what you are asking. However, that last commit should fix your other case now too.

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 11, 2018

I am afraid I don't understand what you are asking. However, that last commit should fix your other case now too.

I installed your fix, but it doesn't work for me.
Let me explain, what i try to do.

I have installed a mopidy, mopidy-spotify, all dependencies and WEB ui "Iris". This all together works fine at all.

What i want to do is starting spotify with MPC command line.

sudo systemctl restart mopidy
pi@box:~ $ sudo mpc -h box -p 6601 add "spotify:artist:0oLWXBuoO0namCRC2oji0q"
pi@box:~ $ sudo mpc -h box -p 6601 clear
volume:100%   repeat: off   random: off   single: off   consume: off
pi@box:~ $ sudo mpc -h box -p 6601 add "spotify:playlist:37i9dQZF1DX0XUsuxWHRQd"
error adding spotify:playlist:37i9dQZF1DX0XUsuxWHRQd: directory or file not found
pi@box:~ $ sudo mpc -h box -p 6601 add "spotify:user:spotify:playlist:37i9dQZF1DX0XUsuxWHRQd"
error adding spotify:user:spotify:playlist:37i9dQZF1DX0XUsuxWHRQd: directory or file not found

you see, adding artist works fine. albums and tracks too.
but when i try to add a playlist with MPC, i get an error.

for own testing, i removed mopidy-spotify (not his dependencies) from my box. i restarted mopidy. the functionality of MPC was the same like before, everything ok but playlists don't work.

so the question is, does MPC use mopidy-spotify for interpeting Spotify URIs?? or does MPC use pyspotify2.0.5 and libspotify12 to do this???

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2018

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2018

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 13, 2018

Damn, i checked the deps and found a second version of mopidy-spotify.
To remove all versions i first did "sudo apt-get remove mopidy-spotify" and then i manually removed "/usr/local/lib/python2.7/dist-packages/Mopidy_Spotify-3.1.0-py2.7.egg".

After that, mopidy told me, that the extension mopidy-spotify is missing.
Ok.

Then i started installation of the master branch.

pi@box:~ $ sudo apt-get remove mopidy-spotify                               Reading package lists... Done
Building dependency tree
Reading state information... Done
Package 'mopidy-spotify' is not installed, so not removed
0 upgraded, 0 newly installed, 0 to remove and 2 not upgraded.
pi@box:~ $ sudo systemctl restart mopidy
pi@box:~ $ sudo apt-get install mopidy-spotify
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following NEW packages will be installed:
  mopidy-spotify
0 upgraded, 1 newly installed, 0 to remove and 2 not upgraded.
Need to get 0 B/23.6 kB of archives.
After this operation, 112 kB of additional disk space will be used.
WARNING: The following packages cannot be authenticated!
  mopidy-spotify
Install these packages without verification? [y/N] y
Selecting previously unselected package mopidy-spotify.
(Reading database ... 144958 files and directories currently installed.)
Preparing to unpack .../mopidy-spotify_3.1.0-0mopidy1_all.deb ...
Unpacking mopidy-spotify (3.1.0-0mopidy1) ...
Setting up mopidy-spotify (3.1.0-0mopidy1) ...
pi@box:~ $ sudo systemctl restart mopidy

At this point i manually overwrote the files in "/usr/lib/python2.7/dist-packages/mopidy_spotify/" with all the files from "kingosticks:fix/web_api_playlists/mopidy_spotify"...

pi@box:~ $ sudo systemctl restart mopidy

pi@box:~ $ mpc -h lianbox -p 6601 add spotify:user:spotify:playlist:37i9dQZF1DWUVpAXiEPK8P
error adding spotify:user:spotify:playlist:37i9dQZF1DWUVpAXiEPK8P: directory or file not found

pi@box:~ $ mpc -h lianbox -p 6601 clear
volume:100%   repeat: off   random: off   single: off   consume: off

pi@box:~ $ sudo mopidyctl deps
Running "/usr/bin/mopidy --config /usr/share/mopidy/conf.d:/etc/mopidy/mopidy.conf deps" as user mopidy
/usr/local/lib/python2.7/dist-packages/pylast/__init__.py:51: UserWarning: You are using pylast with Python 2. Pylast will soon be Python 3 only. More info: https://github.com/pylast/pylast/issues/265
  UserWarning,
Executable: /usr/bin/mopidy
Platform: Linux-4.14.71-v7+-armv7l-with-debian-9.4
Python: CPython 2.7.13 from /usr/lib/python2.7
Mopidy: 2.2.0 from /usr/lib/python2.7/dist-packages
Mopidy-Local-Images: 1.0.0 from /usr/local/lib/python2.7/dist-packages
  Mopidy>=1.1: 2.2.0 from /usr/lib/python2.7/dist-packages
  setuptools: 33.1.1 from /usr/lib/python2.7/dist-packages
  Pykka>=1.1: 1.2.1 from /usr/lib/python2.7/dist-packages
  uritools>=1.0: 2.2.0 from /usr/local/lib/python2.7/dist-packages
    ipaddress; python_version == "2.7": 1.0.17 from /usr/lib/python2.7/dist-packages
Mopidy-Spotify: 3.1.0 from /usr/lib/python2.7/dist-packages
  Mopidy>=2.0: 2.2.0 from /usr/lib/python2.7/dist-packages
  Pykka>=1.1: 1.2.1 from /usr/lib/python2.7/dist-packages
  pyspotify>=2.0.5: 2.0.5 from /usr/lib/python2.7/dist-packages
    cffi>=1.0.0: 1.9.1 from /usr/lib/python2.7/dist-packages
  requests>=2.0: 2.12.4 from /usr/lib/python2.7/dist-packages
Mopidy-Iris: 3.27.1 from /usr/local/lib/python2.7/dist-packages
  tornado<5.0,>=3.2: 4.4.3 from /usr/lib/python2.7/dist-packages
  setuptools>=3.3: 33.1.1 from /usr/lib/python2.7/dist-packages
  ConfigObj>=5.0.6: 5.0.6 from /usr/local/lib/python2.7/dist-packages
    six: 1.10.0 from /usr/lib/python2.7/dist-packages
  requests>=2.0.0: 2.12.4 from /usr/lib/python2.7/dist-packages
  Mopidy>=2.0: 2.2.0 from /usr/lib/python2.7/dist-packages
  Mopidy-Local-Images>=1.0: 1.0.0 from /usr/local/lib/python2.7/dist-packages
    Mopidy>=1.1: 2.2.0 from /usr/lib/python2.7/dist-packages
    setuptools: 33.1.1 from /usr/lib/python2.7/dist-packages
    Pykka>=1.1: 1.2.1 from /usr/lib/python2.7/dist-packages
    uritools>=1.0: 2.2.0 from /usr/local/lib/python2.7/dist-packages
      ipaddress; python_version == "2.7": 1.0.17 from /usr/lib/python2.7/dist-packages
  pylast>=1.6.0: 2.4.0 from /usr/local/lib/python2.7/dist-packages
    six: 1.10.0 from /usr/lib/python2.7/dist-packages
GStreamer: 1.10.4.0 from /usr/lib/python2.7/dist-packages/gi
  Detailed information:
    Python wrapper: python-gi 3.22.0
    Relevant elements:
      Found:
        uridecodebin
        souphttpsrc
        appsrc
        alsasink
        osssink
        oss4sink
        pulsesink
        id3demux
        id3v2mux
        lamemp3enc
        mad
        mpegaudioparse
        mpg123audiodec
        vorbisdec
        vorbisenc
        vorbisparse
        oggdemux
        oggmux
        oggparse
        flacdec
        flacparse
        shout2send
      Not found:
        flump3dec

What did i wrong?
Was my way the correct way to get your fix running?

If not, can you explain, me how to get your fix installed correctly?
MANY THANKS!

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2018

Not really. There's no real correct way, installing development code (which this obviously is) for use with the service isn't something we support. But if I were doing it, I would

  • 'apt-get remove mopidy-spotify'
  • 'apt-get install python-spotify'
  • download the mopidy-spotify files from the fix branch somewhere. Goto that directory and run 'sudo pip install .'
  • note the full stop at the end
  • run 'sudo mopidyctl deps' to verify it can find mopidy-spotify in the directory
@princemaxwell

This comment has been minimized.

Copy link

commented Oct 13, 2018

@kingosticks AHHHHHHH!
I got it! You will never believe, what i had to do...
I used an old client_id and client_secret from developer.spotify.com and i had to renew this on mopidy.com/authenticate/ and then playlists are working FINE.

Thanks a lot, this was my fault. But i never thought about about that...
the whole time i wasn't using the web API ;-) since now!

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 13, 2018

@kingosticks
ok, it works now, but the start of mopidy is very very slow.

INFO     Starting Mopidy 2.2.0
INFO     Loading config from builtin defaults
INFO     Loading config from /home/pi/.config/mopidy/mopidy.conf
INFO     Loading config from command line options
INFO     Enabled extensions: iris, mpd, http, stream, spotify, m3u, softwaremixer, file, local-images, local
INFO     Disabled extensions: none
INFO     Starting Mopidy mixer: SoftwareMixer
INFO     Starting Mopidy audio
INFO     Starting Mopidy backends: StreamBackend, M3UBackend, FileBackend, LocalBackend, SpotifyBackend
INFO     Audio output set to "alsasink"
INFO     No local library metadata cache found at /home/pi/.local/share/mopidy/local/library.json.gz. Please run `mopidy local scan` to index your local music library. If you do not have a local music collection, you can disable the local backend to hide this message.
INFO     Loaded 0 local tracks using json
INFO     Logged in to Spotify in offline mode
INFO     Logged in to Spotify in online mode
INFO     Logged into Spotify Web API as spotifyuser
INFO     Loaded 47 playlists
INFO     Refresh Playlists took 14812ms
INFO     Starting Mopidy core
INFO     Starting Mopidy frontends: IrisFrontend, MpdFrontend, HttpFrontend
INFO     Starting Iris 3.27.1
INFO     MPD server running at [::ffff:0.0.0.0]:6601
INFO     Starting GLib mainloop
INFO     HTTP server running at [::ffff:0.0.0.0]:6680

Why does mopidy-spotify load my playlists, whats takes 15seconds.
Can i skip this step?

It take 2 more minutes until Mopidy Web UI (Iris) is working.

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2018

It loads all your playlists yes. It could do this in the background like libspotify did.

As to why iris takes so long I'm not sure. Perhaps it's doing a lot of extra playlist lookups. But once that's all done it should be cached and it should be quick to browse, much more usable than deferring loading until you select the playlist. Typically mopidy is starting as you system starts so it it takes 15 seconds I don't think that's a big issue.

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 13, 2018

When i replace your branch with the master branch, mopidy is starting in 5 seconds and Iris is available after 10-15 seconds.
Why does it load all my playlists? My intention is to load a playlist via mpc...

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2018

Yeh, and none of the playlists work so that's really not an interesting data point.

Loading all your playlists once at startup caches the data and makes selecting a playlist later much, much faster. If we instead only load them on demand then you'll have to wait a few seconds in your client, much longer for big playlists, and its a much worse user experience.

I will do some testing with iris myself and see what it's doing differently to the other clients.

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 16, 2018

@princemaxwell I moved the initial load into a separate thread. I'm not sure it's a good idea. Out of interest, what situation do you have where you need Mopidy to start that quickly? Is it a real issue or just something you have noticed because you are testing?

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 16, 2018

@kingosticks i have build a music box for the kids with RFID card reader to start music by swiping cards easily.

The box starts and a start sound appears. this should be the signal for the kids, that the box is ready. If configured the startsound.service to start after Mopidy.service. But the sound is played minutes before Mopidy is ready to use.

So the first problem is, that Mopidy needs much more time to start completely with this playlist_fix as without.

I use the MPD from Mopidy to play local MP3, livestreams and Spotify.

The second problem is, that the MP3 and livestream loading is slow with MPD + Mopidy.
When I only use MPD without Mopidy, MP3 and livestreams are loading fast.

I hope that I explained it correct now...

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 16, 2018

OK thanks. In general I would suggest you implement a more robust solution that polls mopidy and waits for it to respond before playing the sound. With the latest code it will appear ready much earlier but ultimately any Spotify playlist actions won't be successful until the initial (now background) playlist loading completes.

As to the second issue - I still don't understand but it sounds like a very separate issue so let's not discuss that here.

@princemaxwell

This comment has been minimized.

Copy link

commented Oct 16, 2018

@kingosticks Feedback: I tested your last commit. Mopidy now starts in 8 seconds. That's great.
My problems with MP3 and livestreams are not a problem of mopidy-spotify, because they occur even if mopidy-spotify is not installed.
So, that is a problem of mopidy...

@girst

This comment has been minimized.

Copy link

commented Oct 20, 2018

I don't think the async loading of playlists is that great. It makes it much harder to script the execution of a playlist (e.g. wait for mopidy to be started, then load and start a playlist).

What good comes from a fast startup when it isn't usable afterwards?

@kingosticks

This comment has been minimized.

Copy link
Member Author

commented Oct 20, 2018

Ideally you will wait for the playlists loaded event to fire, if whatever you want to do involves playlists.

Edit: but personally I do agree. Adding multiple threads into the mix for this tiny gain is not worth the trouble. I don't intend to keep this commit.

@girst

This comment has been minimized.

Copy link

commented Oct 20, 2018

I don't intend to keep this commit.

Good to hear ;-P (snark aside, I'm very pleased and thankful that you to have taken on the modernization of this program I use every day)!

Ideally you will wait for the playlists loaded event to fire

yes, I'm doing it the lazy way to just wait for MPD server running at to be printed to stderr before continuing my script (and have written a tool for this (among other things) even).

(edited to fix formatting)

@ikcalB

This comment has been minimized.

Copy link

commented Jul 12, 2019

any progress merging this into master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.