beetbox
`fetchart.ART_SOURCES` does not support item assignment
Problem
Running this command in verbose (-vv) mode:
$ beet -vv import -LI 'id::^9664$'
Led to this problem:
user configuration: /home/sarunas/.config/beets/config.yaml
data directory: /home/sarunas/.config/beets
plugin paths:
fetchart: google: Disabling art source due to missing key
inline: adding item field has_lyrics
inline: adding item field label_or_albumartist
inline: adding item field singleton_track_artist
inline: adding item field track_artist
inline: adding item field album_name
inline: adding item field track_identification
inline: adding item field withdrawn
inline: adding album field label_or_albumartist
inline: adding album field multiple_artists
Sending event: pluginload
Traceback (most recent call last):
File "/home/sarunas/.local/bin/beet", line 5, in <module>
main()
File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1878, in main
_raw_main(args)
File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1853, in _raw_main
subcommands, plugins, lib = _setup(options, lib)
File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1693, in _setup
plugins.send("pluginload")
File "/home/sarunas/repo/beets/beets/plugins.py", line 634, in send
result = handler(**arguments)
File "/home/sarunas/repo/beets/beets/plugins.py", line 206, in wrapper
return func(*args, **kwargs)
File "/home/sarunas/repo/beetcamp/beetsplug/bandcamp/__init__.py", line 178, in loaded
fetchart.ART_SOURCES[self.data_source] = BandcampAlbumArt
TypeError: 'set' object does not support item assignment
Setup
- OS: Arch Linux x86_64 / Linux 6.14.6-arch1-1
- Python version: Python 3.9.20
- beets version: most recent commit on master, 79c87e58
- Turning off plugins made problem go away (yes/no): yes
My configuration (output of beet config) is:
Relevant configuration which triggers the error
plugins: bandcamp
bandcamp:
art: yes
Setting art to no fixes the issue.
plugins: bandcamp
bandcamp:
art: no
Context
Due to lack of an ability for plugins to register their own fetchart art sources, beetcamp has been relying on this patch to force its art source in:
class BandcampAlbumArt(..., fetchart.RemoteArtSource):
NAME = "Bandcamp"
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
self.config = self._config
def get(self, album: AlbumInfo, *_: Any) -> Iterable[fetchart.Candidate]:
"""Return the url for the cover from the bandcamp album page.
This only returns cover art urls for bandcamp albums (by id).
"""
url = album.mb_albumid
if not self.from_bandcamp(url):
self._info("Not fetching art for a non-bandcamp album URL")
else:
with self.handle_error(url):
if image := self.guru(url).image:
yield self._candidate(
url=image, match=fetchart.Candidate.MATCH_EXACT
)
class BandcampPlugin:
data_source = "bandcamp"
def __init__(self) -> None:
...
if self.config["art"]:
self.register_listener("pluginload", self.loaded)
def loaded(self) -> None:
"""Add our own artsource to the fetchart plugin."""
for plugin in plugins.find_plugins():
if isinstance(plugin, fetchart.FetchArtPlugin):
fetchart.ART_SOURCES[self.data_source] = BandcampAlbumArt
fetchart.SOURCE_NAMES[BandcampAlbumArt] = self.data_source
fetchart.SOURCES_ALL.append(self.data_source)
bandcamp_fetchart = BandcampAlbumArt(self._log, self.config)
plugin.sources = [bandcamp_fetchart, *plugin.sources]
break
Seems like these global variables have been adjusted in #5716 which made it explode. I myself rely on this art source every day; I think people who use this plugin generally do, too.
@beetbox/maintainers what do you reckon would be the best way to go about this longer term? I understand this is a private API so I wouldn't expect adjustments to be made in beets source - I'll do it on my end. On the other hand, the fix will involve more of the same - patching internal fetchart API. Have we ever considered making art source registration public?
Sorry, I added an entry to the changelog in that PR, but maybe I should have been more vocal about it.
Have we ever considered making art source registration public?
Not that I know of, but in principle, this seems like a reasonable extension point (it's a bit unclear how to do this exactly: Ideally, there would be a registration method provided by the plugin, maybe with beets.plugins acting as a middle man. It should be general enough to cover arbitrary plugins; e.g. the lyrics plugin might also benefit from an extension API).
It would have to include more than just registration: The relevant API also includes the entire class structure of fetchart, i.e. the Candidate and the ArtSource subclasses.
Generally, I don't think we have a very clear specification of our API and any stability promises, and we have definitely broken things all over the place many times. I feel like there's some benefit to allowing this, enabling larger refactoring without spending too much of our limited developer bandwith on backwards compatibility.
Before extending API promises, we should also be somewhat certain that the corresponding implementation is in good shape and not overly likely to see any major refactoring soon.
I also feel like we should try to verify APIs that we promise. Maybe there's a way to define an API in something like a .pyi file and check that in CI?
Not that I know of, but in principle, this seems like a reasonable extension point (it's a bit unclear how to do this exactly: Ideally, there would be a registration method provided by the plugin, maybe with beets.plugins acting as a middle man. It should be general enough to cover arbitrary plugins; e.g. the lyrics plugin might also benefit from an extension API). It would have to include more than just registration: The relevant API also includes the entire class structure of fetchart, i.e. the Candidate and the ArtSource subclasses.
Having a registry akin to how plugins are currently managed does seem like a solid idea. However, I’m not sure if introducing additional coupling between beets core and plugins is something we want, given existing concerns around this. A better middle ground might be to allow plugins to define and manage their own registries, using utilities provided by the core, without enforcing a core-managed solution.
Generally, I don't think we have a very clear specification of our API and any stability promises, and we have definitely broken things all over the place many times. I feel like there's some benefit to allowing this, enabling larger refactoring without spending too much of our limited developer bandwith on backwards compatibility.
Unfortunately, we don’t seem to adhere to semantic versioning principles very strictly. Ideally, breaking changes should be reserved for major version bumps, but from what I’ve seen, we haven’t really made use of that — we’re still on v2, despite several backwards-incompatible changes. Part of the issue might be a lack of clarity around what constitutes a public API or which parts of the codebase are considered "stable" versus "internal."
Originally, beets was intended as more of a standalone CLI application, but it's increasingly evolving into a broader toolbox or package. As this shift continues, defining and communicating a clearer boundary between public and internal APIs might become more important.
I also feel like we should try to verify APIs that we promise. Maybe there's a way to define an API in something like a .pyi file and check that in CI?
It should be possible to check our codebase against .pyi interface definitions using mypy. That could help verify that our implementation matches what we consider to be the public API.
I think this might be a bit too much overhead to maintain by hand, though. Normally, I’d generate .pyi files automatically (e.g. with mypy's stubgen), and then on each version bump, compare the generated stubs to see if there were any breaking changes.
That approach assumes we have a consistent versioning strategy, which we kind of don’t right now. So maybe this only really works if we also get clearer about what we define as public API and treat versioning accordingly.
