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

Disable plugin options requiring a missing dependency #1963

Closed
Kraymer opened this issue Apr 27, 2016 · 6 comments
Closed

Disable plugin options requiring a missing dependency #1963

Kraymer opened this issue Apr 27, 2016 · 6 comments
Labels
feature features we would like to implement

Comments

@Kraymer
Copy link
Contributor

Kraymer commented Apr 27, 2016

When a library related to a plugin option (langdetect for translate, bs4 for google lyrics backend, etc) is missing, it should be detected at the plugin init and the offending option should be disabled with a message printed once pointing to the docs.
Currently we have an error/warning for each song.

@Kraymer Kraymer added the feature features we would like to implement label Apr 27, 2016
@jackwilsdon
Copy link
Member

It would be cool if plugins themselves could tell beets core about the issue and beets would disable the plugin automatically. I wonder if this is possible?

@GuilhermeHideki
Copy link
Member

maybe with __import__ / importlib help. You would try to import and logs a message if can't import something.

@wisp3rwind
Copy link
Member

This is exactly what beets already does (see https://github.com/beetbox/beets/blob/master/beets/plugins.py#L267 which also catches any ImportError. Raising any error during plugin init will effectively disable the plugin). The problem with lyrics is different, though. The lack of either of these modules should not shutdown the whole plugin, but only deactivate the corresponding sources. The recent patch does that, but it will also print a warning every time lyrics are fetched, which messes up the UI during imports. That's what Kraymer was about, the plugin would need some refactoring to print that warning only once.

@GuilhermeHideki
Copy link
Member

adding something like this at the top would work / is too hacky?

try:
  import langdetect
except ImportError:
  disable_source('bing')
  warn_user()

@wisp3rwind
Copy link
Member

The warning should not print when the source is not enabled through the config, though, as that combination may aswell be intentional. Have a look at LyricsPlugin.__init__, the google source does that when no API key is given.

@wisp3rwind
Copy link
Member

Fixed in #1968

wisp3rwind pushed a commit that referenced this issue Apr 28, 2016
lyrics: clean up import handling and source removal (Fixes #1963)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement
Projects
None yet
Development

No branches or pull requests

4 participants