python-musicbrainzngs needs a replacement

[Louson]

As explained in #4651 , the musicbrainz python library python-musicbrainzngs receives no update.

It is used in beets auto-tagging and in several plugins.

As @sampsyo truly says, it was created when the only output format was xml and has a lot of parsing code that can be deleted. Also, after digging in it, it reveals some lack of conception, using sometimes oriented object programming and sometimes global variables.

It can indeed be replaced by a much simpler library. Of course, this implies to find who and when. But first, two question have to be answered:

Q1. Implement the library inside this project or as an independent project

Both have some pros and cons.

The cons about implementing it inside beets is that it will add some maintainance work to the beets developpers.
The cons about an independant project is that it requires at least one developer who actively maintains it, with the risk for it to be abandoned again.

I think at this stage it is better to implement into beets project unless we know some others projects that use this and who will appreciate to share the effort.

Q2. JSON abstraction or simple pass-through

We have two options:

• the library is a simple pass-through and it is up to the caller to parse the json
• the library has an interface that hides the JSON. The advantage is we don't have to change the caller when the API changes. It gives more stability over time

[sampsyo]

I'd be open to any and all opinions on this general direction, but here are two gut reactions:

• There is an "if you build it, they will come" aspect to this kind of library. It could make a lot of sense to start the project as an independent library under the Beetbox org. It seems like many projects would at least potentially be interested in using it, and the more (non-beets) users there are, the more we can leverage development effort to build a high-quality library we can all use.
• A simple pass-through of the JSON responses seems very much the right thing to me. This would avoid the need to make changes to the library to adapt when new stuff is added to the API. (This is avoids the trap that the current library is in: new features have been added to the API, but adding support for them in the library requires boring data-munging work to expose the new data.) And breaking changes to the MB API are rare and well-publicized, so it makes sense to externalize this risk to end applications.

[RollingStar]

A simple pass-through of the JSON responses seems very much the right thing to me. This would avoid the need to make changes to the library to adapt when new stuff is added to the API. (This is avoids the trap that the current library is in: new features have been added to the API, but adding support for them in the library requires boring data-munging work to expose the new data.) And breaking changes to the MB API are rare and well-publicized, so it makes sense to externalize this risk to end applications.

How would this look? I agree in principle that it should be something like response['album'] rather than making a bespoke Album class that has to be updated with every MB update. But I wonder if even a light-touch passthrough would be better off in its own library, to let people use it outside of beets. There's MB auth, rate limits, and the possibility of querying your own local MB server to consider.

[sampsyo]

Oh yes! I completely agree with this, if I didn't make that clear:

even a light-touch passthrough would be better off in its own library, to let people use it outside of beets

I would love to develop this as a separate library that lives on PyPI. Publishing the library (even if it's a simple pass-through thing) would really help leverage the support of the community.

[Louson]

I would love to develop this as a separate library that lives on PyPI. Publishing the library (even if it's a simple pass-through thing) would really help leverage the support of the community.

I am not a python expert. I won't be able to help you on packaging and maintaining. But I can help you on the development. So far I have inspected the musicbrainzngs. I have started something based on it, including:

• dropping xml parsing stuff
• changing the authentication process, including oauth2
• setting https as default
• changing the way the format (json or xml) is defined
• changing tests to match https, json and authentication

[Serene-Arc]

I might be interested in rewriting the library. It seems like a fun project.

[Serene-Arc]

Ah nevermind! I didn't notice the date for this.

[Louson]

There is an extra question about credentials (password or oauth). The former library used to store the login/password. In my opinion, the library should not do that and the credentials should only be managed by the caller. What is your opinion ?

[sampsyo]

Right; a clean-slate implementation would probably want to be OAuth-only (or at least OAuth-first).

[Louson]

Hello, here is a suggestion https://gitlab.com/Louson/python-musicbrainzez/-/merge_requests/1

It looks huge but most of it is duplicating xml tests in json.

Can you have a look ?
It lacks packaging and documentation fix, and probably some organization.

I kept the parsing in a dedicated subfolder, but it is not hold by default.
The JSON API is requested by default, but the xml API can be requested in any request.
I renamed the library. I am not sure this is relevant but this actually gives the possibility to create a musicbrainzngs/musicbrainz.py that adds backward compatibility.
I disabled the password storage and added oauth2

[RollingStar]

Did a semi thorough skim of the non test parts. Skimmed the tests. Thank you for this!

Good "port" of the functionality which makes it easier to review.

Once this review is done, you may want to make more changes that make the code easier to read/maintain.

User agent should? be stored in a consts.py or similar, then "import USERAGENT from consts"

Would it make sense to abbreviate the import?

import musicbrainzez as mez

I tried finding best practice. It looks like pandas imports "pandas", but it's a lot easier to write than "musicbrainzez"

Should this change?
classifiers=[
"Development Status :: 5 - Production/Stable",

For readers, make_barcode_request and the funcs around it are removed because they come back in another file.

It would have been easier to review this if the test data was in its own separate PR submitted after the core PR was done.

Same with having tests in their own PR.

[Louson]

Hello, thanks for the review.

Actually, every commit is very singular and would require its own PR.
It is more relevant to check it commit by commit skipping examples and tests.

This is not an official PR, but only a suggestion for someone who can carry the package.

[Louson]

Hello @sampsyo, would you like to also give it a view ?

[Louson]

Hello, I might have some time for this. Do you have any idea how to go further ?

[Louson]

Hello, I archived the project as I made more interest to the rework https://gitlab.com/mbzero/python-mbzero after @sampsyo 's suggestions. Unfortunately #4936 does not apply because of the massive lint done after my push. I had a look and it didn't look easy to update the PR. I must work on it, but perhaps it's possible to have some review before I spend time in it ? Thanks

[RollingStar]

That's okay, I was just looking for a newer Python API for Musicbrainz.

[Louson]

Would you like to help ? There is probably a lot of things to do, mainly tests before getting it stable

[RollingStar]

Not in the next few months, sorry.

[Louson]

Hello, I have updated the repository https://gitlab.com/mbzero/python-mbzero
Please make feedbacks if you try it