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

Revive Beatport plugin with support for OAuth API (#1989) #2067

Merged
merged 5 commits into from Jun 21, 2016

Conversation

Projects
None yet
5 participants
@jbaiter
Contributor

jbaiter commented Jun 21, 2016

This restores the previously removed Beatport plugin, this time around with support for their API version 3, which requires OAuth authentication with all endpoints.

The process of obtaining the authentication token and secret is modeled after the way the discogs plugin handles this, i.e. the user is asked to visit a website, enter her credentials and then paste the displayed string into the terminal. The plugin then extracts the token/secret and stores it in a file in the configuration directory.

I have not yet tested the integration with beets, so I would welcome any curious testers to try it out.

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Jun 21, 2016

Member

Yay! Fantastic -- this looks great after a quick review.

For the documentation, shall we revive the old documentation file too and add notes about the new OAuth workflow?

And not that it's urgent, but some of the methods in BeatportClient could use docstrings.

Member

sampsyo commented Jun 21, 2016

Yay! Fantastic -- this looks great after a quick review.

For the documentation, shall we revive the old documentation file too and add notes about the new OAuth workflow?

And not that it's urgent, but some of the methods in BeatportClient could use docstrings.

jbaiter added some commits Jun 21, 2016

@jbaiter

This comment has been minimized.

Show comment
Hide comment
@jbaiter

jbaiter Jun 21, 2016

Contributor

For the documentation, shall we revive the old documentation file too and add notes about the new OAuth workflow?

Yes, that's the most sensible way to go, since the OAuth thing is the only thing that has changed since the removal. I just added the old documentation file with a new paragraph on the OAuth workflow.

And not that it's urgent, but some of the methods in BeatportClient could use docstrings.

Done! :-)

Contributor

jbaiter commented Jun 21, 2016

For the documentation, shall we revive the old documentation file too and add notes about the new OAuth workflow?

Yes, that's the most sensible way to go, since the OAuth thing is the only thing that has changed since the removal. I just added the old documentation file with a new paragraph on the OAuth workflow.

And not that it's urgent, but some of the methods in BeatportClient could use docstrings.

Done! :-)

@sampsyo sampsyo merged commit 7f98b45 into beetbox:master Jun 21, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

sampsyo added a commit that referenced this pull request Jun 21, 2016

Merge pull request #2067 from jbaiter/beatport
Revive Beatport plugin with support for OAuth API (#1989)

sampsyo added a commit that referenced this pull request Jun 21, 2016

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Jun 21, 2016

Member

Wahoo! That was fast! And the new text in the docs looks great. I'm happy we get to have a Beatport plugin again! 🚀

Member

sampsyo commented Jun 21, 2016

Wahoo! That was fast! And the new text in the docs looks great. I'm happy we get to have a Beatport plugin again! 🚀

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

I'm up to date with the latest master branch, but am still getting ** plugin beatport not found beets version 1.3.18 no plugins loaded and nothing happens (i.e. no beatport OAuth website or anything). Any ideas what I am doing wrong?

brianroach commented Jun 22, 2016

I'm up to date with the latest master branch, but am still getting ** plugin beatport not found beets version 1.3.18 no plugins loaded and nothing happens (i.e. no beatport OAuth website or anything). Any ideas what I am doing wrong?

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Jun 22, 2016

Member

Hmm, that's odd -- maybe you need to uninstall and reinstall? Things seem to work fine here.

Member

sampsyo commented Jun 22, 2016

Hmm, that's odd -- maybe you need to uninstall and reinstall? Things seem to work fine here.

@jackwilsdon

This comment has been minimized.

Show comment
Hide comment
@jackwilsdon

jackwilsdon Jun 22, 2016

Member

That's peculiar. What's the output from git rev-parse HEAD inside the beets directory?

Member

jackwilsdon commented Jun 22, 2016

That's peculiar. What's the output from git rev-parse HEAD inside the beets directory?

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

@sampsyo I actually did just uninstall & re-install because I was getting that error initially, but got it again after a clean install.

@jackwilsdon 0b17666b55f498e5f5b74bb5021da7c93b577690

brianroach commented Jun 22, 2016

@sampsyo I actually did just uninstall & re-install because I was getting that error initially, but got it again after a clean install.

@jackwilsdon 0b17666b55f498e5f5b74bb5021da7c93b577690

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

@sampsyo what does your config file look like as far as options go?

brianroach commented Jun 22, 2016

@sampsyo what does your config file look like as far as options go?

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Jun 22, 2016

Member

This is the whole config file:

plugins: beatport
Member

sampsyo commented Jun 22, 2016

This is the whole config file:

plugins: beatport
@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

@sampsyo @jackwilsdon scratch that - I figured it out. I re-installed to a different folder and forgot to update the plugin path in config.yaml - oops!

brianroach commented Jun 22, 2016

@sampsyo @jackwilsdon scratch that - I figured it out. I re-installed to a different folder and forgot to update the plugin path in config.yaml - oops!

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

@sampsyo so since there's no args to pass with it then does it just fetch all available metadata that beatport has?

brianroach commented Jun 22, 2016

@sampsyo so since there's no args to pass with it then does it just fetch all available metadata that beatport has?

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Jun 22, 2016

Member

Yes.

Member

sampsyo commented Jun 22, 2016

Yes.

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

I just ran it, go the string in the browser but then got this: Traceback (most recent call last): File "/Users/anodigital/.pyenv/versions/2.7.10/bin/beet", line 9, in <module> load_entry_point('beets==1.3.18', 'console_scripts', 'beet')() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/__init__.py", line 1250, in main _raw_main(args) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/__init__.py", line 1240, in _raw_main subcommand.func(lib, suboptions, subargs) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/commands.py", line 967, in import_func import_files(lib, paths, query) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/commands.py", line 944, in import_files session.run() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 320, in run pl.run_parallel(QUEUE_SIZE) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/util/pipeline.py", line 301, in run out = self.coro.send(msg) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/util/pipeline.py", line 183, in coro func(*(args + (task,))) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 1264, in lookup_candidates task.lookup_candidates() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 591, in lookup_candidates autotag.tag_album(self.items, search_ids=self.search_ids) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/autotag/match.py", line 439, in tag_album search_album, va_likely) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/autotag/hooks.py", line 582, in album_candidates out.extend(plugins.candidates(items, artist, album, va_likely)) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/plugins.py", line 355, in candidates out.extend(plugin.candidates(items, artist, album, va_likely)) File "/Users/anodigital/github/beets/beetsplug/beatport.py", line 344, in candidates return self._get_releases(query) File "/Users/anodigital/github/beets/beetsplug/beatport.py", line 396, in _get_releases for x in self.client.search(query).results] AttributeError: 'BeatportPlugin' object has no attribute 'client'

brianroach commented Jun 22, 2016

I just ran it, go the string in the browser but then got this: Traceback (most recent call last): File "/Users/anodigital/.pyenv/versions/2.7.10/bin/beet", line 9, in <module> load_entry_point('beets==1.3.18', 'console_scripts', 'beet')() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/__init__.py", line 1250, in main _raw_main(args) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/__init__.py", line 1240, in _raw_main subcommand.func(lib, suboptions, subargs) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/commands.py", line 967, in import_func import_files(lib, paths, query) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/ui/commands.py", line 944, in import_files session.run() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 320, in run pl.run_parallel(QUEUE_SIZE) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/util/pipeline.py", line 301, in run out = self.coro.send(msg) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/util/pipeline.py", line 183, in coro func(*(args + (task,))) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 1264, in lookup_candidates task.lookup_candidates() File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/importer.py", line 591, in lookup_candidates autotag.tag_album(self.items, search_ids=self.search_ids) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/autotag/match.py", line 439, in tag_album search_album, va_likely) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/autotag/hooks.py", line 582, in album_candidates out.extend(plugins.candidates(items, artist, album, va_likely)) File "/Users/anodigital/.pyenv/versions/2.7.10/lib/python2.7/site-packages/beets/plugins.py", line 355, in candidates out.extend(plugin.candidates(items, artist, album, va_likely)) File "/Users/anodigital/github/beets/beetsplug/beatport.py", line 344, in candidates return self._get_releases(query) File "/Users/anodigital/github/beets/beetsplug/beatport.py", line 396, in _get_releases for x in self.client.search(query).results] AttributeError: 'BeatportPlugin' object has no attribute 'client'

@jackwilsdon

This comment has been minimized.

Show comment
Hide comment
@jackwilsdon

jackwilsdon Jun 22, 2016

Member

Do you think you could open a new issue with all of the necessary information @brianroach?

Member

jackwilsdon commented Jun 22, 2016

Do you think you could open a new issue with all of the necessary information @brianroach?

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach commented Jun 22, 2016

@jackwilsdon will do.

@jbaiter

This comment has been minimized.

Show comment
Hide comment
@jbaiter

jbaiter Jun 22, 2016

Contributor

This is a bad naming confusion on my part, I had the client stored under beatport_api but accessed it with client. I just pushed a fix (plus one for another unrelated bug) to my branch and will open a PR. Sorry!

Contributor

jbaiter commented Jun 22, 2016

This is a bad naming confusion on my part, I had the client stored under beatport_api but accessed it with client. I just pushed a fix (plus one for another unrelated bug) to my branch and will open a PR. Sorry!

@brianroach

This comment has been minimized.

Show comment
Hide comment
@brianroach

brianroach Jun 22, 2016

@jbaiter ah, no worries. thanks!

brianroach commented Jun 22, 2016

@jbaiter ah, no worries. thanks!

@willgrnr

This comment has been minimized.

Show comment
Hide comment
@willgrnr

willgrnr Jun 28, 2016

I am so glad this has finally made a comeback - thanks @jbaiter

willgrnr commented Jun 28, 2016

I am so glad this has finally made a comeback - thanks @jbaiter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment