Implement "Create new release on musicbrainz" feature #5300
Conversation
f499389
to
ffcce62
Compare
0ab0a85
to
096356e
Compare
|
Hey @doronbehar, If you're around, have a look at this. Maybe I can remove the "Open with Picard" option now that this here is implemented? |
There was a problem hiding this comment.
Thanks for the PR! I've made a couple comments but I'll also note that a fair amount of the changes in this PR are fairly boilerplate HTTP webserver stuff. Have you considered using something like simple-http-server or another package to deal with this? It would reduce the number of lines in the PR and make it easier to change in the future.
beetsplug/mbsubmit.py
Outdated
| formdata: dict | ||
| """ | ||
| Form data to be submitted to MB. | ||
| """ | ||
|
|
||
| result_release_mbid: Optional[str] = None | ||
| """ | ||
| Contains the release ID returned by MusicBrainz after the release was created. | ||
| """ | ||
|
|
||
| browser_opened: bool = False | ||
| """ | ||
| True when the user has opened the link for this task in the browser. | ||
| """ |
There was a problem hiding this comment.
Docstring comments go above variables. It's only classes and functions that have them below the definition. Confusing, I know
There was a problem hiding this comment.
Do they? See https://peps.python.org/pep-0258/#attribute-docstrings... also, PyCharm correctly picks up the docstrings this way.
docs/plugins/mbsubmit.rst
Outdated
|
|
||
| .. code-block:: console | ||
|
|
||
| $ pip install beets[mbsubmit] |
There was a problem hiding this comment.
With the switch to poetry, this is now outdated. Should be changed to the poetry equivalent.
There was a problem hiding this comment.
Is it?
https://github.com/beetbox/beets/blob/master/docs/plugins/fetchart.rst?plain=1#L12
I can add quotation marks for consistency.
I thought about not pulling int too many new dependencies. I can also use flask instead, since that's already used by a couple other plugins...? |
|
I don't think many dependencies is an issue that is too pressing. If it makes the code easier to read and maintain, that wins out over another dependency in my book. If flask is used by other plugins though, by all means use it! |
|
Thank you for the review! It's now much cleaner and shorter with flask. |
Hey! Personally I won't mind it getting removed, as I'd definitely use this functionality once it will land! It also makes sense to me from a general point of view to remove it, but that needs an approval from the maintainers of beets. Thanks for working on this feature! |
2486114
to
420178a
Compare
|
|
||
| .. code-block:: console | ||
|
|
||
| $ pip install "beets[mbsubmit]" |
There was a problem hiding this comment.
We're now recommending that people install and manage beets with poetry, so if you wouldn't mind changing this, it should be poetry install beets --extras mbsubmit.
| formdata: Dict[str, str] | ||
| """ | ||
| Form data to be submitted to MB. | ||
| """ | ||
|
|
||
| browser_opened: bool = False | ||
| """ | ||
| True when the user has opened the link for this task in the browser. | ||
| """ | ||
|
|
||
| result_release_mbid: Optional[str] = None | ||
| """ | ||
| Contains the release ID returned by MusicBrainz after the release was created. | ||
| """ |
There was a problem hiding this comment.
Comments go above variables, please.
| def _wait_for_condition(self, condition: Callable): | ||
| t = threading.current_thread() | ||
| while not condition(): | ||
| time.sleep(0.5) |
There was a problem hiding this comment.
Is this necessary? This is a blocking call on the other thread, so is there any penalty to removing it?
Description
Closes #1866.
Implement support to create new releases on musicbrainz from unmatched albums directly.
Screencast.from.2024-06-12.23-37-41.webm
See #5299.
See documentation and comments for details.
If no unmatched album is available for testing, set configuration mbsubmit.threshold to 'strong' and run import with --timid to select the new option.
To Do
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)