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

depend on requests (experimental) #123

Closed
wants to merge 1 commit into from
Closed

Conversation

sampsyo
Copy link
Collaborator

@sampsyo sampsyo commented Sep 13, 2013

Food for thought.

I just thought I'd try to gauge others' opinions on the idea of depending on requests. Since we spend a lot of effort on setting up the HTTP process and handling errors, this turns out to replace quite a bit of code. Interesting!

This seems like it might be a boon for maintainability/reliability, but I'm mainly opening this PR for discussion, not on the expectation that it will be immediately merged. (For beets, we're considering moving to requests for everything else sometime soon, so the extra dependency wouldn't be a problem. But I'm there might be other projects out there for which another dependency would be a pain.)

@JonnyJD
Copy link
Collaborator

JonnyJD commented Sep 15, 2013

We shouldn't rush, since that module is quite new, but it would probably save us from some headaches (like #103)
Looks like linux distrubutions also start to include (version 1.x) of the module. Debian wheezy (current stable) only has 0.12. So people installing a newer musicbrainzngs (like from a PPA) would also need to install a newer requests module.
The required python version is the same as for us: 2.6-3.3.

For isrcsubmit the additional dependency would be fine and could be shipped in the windows and mac packages.
I didn't explicitely test that yet, but requests should work the same for Python 2 and Python 3 unchanged, according to missing 2to3 in setup.py.

This obviously needs some testing. I would especially like to test the encoding mentioned in #103, but I don't have time for that now (TODO).

@alastair
Copy link
Owner

I'd be interested in considering this for an API break release. I would prefer to keep external dependencies to a minimum, but I think requests is one of those things that makes stuff so much easier we may as well go for it.
@JonnyJD I wonder how much change from 0.12->1.0 is due to bug fixes and how much is features. Is there any API breakages? Maybe we could support older versions too by being smart.

@mineo
Copy link
Collaborator

mineo commented Nov 15, 2013

I'm definitely in favor of this, there's just too much magic happening in al the HTTP-related code.

I'm not sure how much we should care about Debian wheezy, after all, they still ship version 0.2 of musicbrainzngs.

@dufferzafar
Copy link
Contributor

If everyone agrees with switching to requests, should I work on finishing this?

@alastair
Copy link
Owner

alastair commented Apr 6, 2015

Sorry for the delay, I've been thinking about this a lot and still can't really come to a decision. I really like requests, but am still a little worried about easily finding the dependency in all the places we are.
I see the original patch is 1.5 years old! I guess the scene has changed quite a bit since then. Can anyone give me an overview of their current distro - how available is requests 1.0 there?

@sampsyo
Copy link
Collaborator Author

sampsyo commented Apr 7, 2015

Looks like Requests 2.x is in Debian Jessie (which will be Debian Stable in a matter of weeks): https://packages.debian.org/sid/python-requests

I'm happy to do this work again for the current version of the library. I'd love to see the library depend on requests, but I understand that taking a dependency is worth careful consideration.

@alastair
Copy link
Owner

alastair commented Apr 7, 2015

Cool - this is promising, except we all know that just because something is almost Stable, it doesn't mean that everyone will immediately upgrade :)
Ubuntu 12.04 is still supported for 1.5 years more, though this is in the same situation as debian oldstable (since we have 14.4), so we could consider both of them as requiring the package from pypi.

I'm definitely happy to include this in our mythical apichange milestone. I'm not sure what that means in practise though - ws/3, 6 months, 12 months?

@sampsyo
Copy link
Collaborator Author

sampsyo commented Apr 7, 2015

Well, the nice thing about stable releases is that this library is stabilized also. 😃 People on an old release will have an old version of musicbrainzngs. If they want to install from pip, then it seems fine that this should also pull in dependencies from pip.

Overall, it seems like we could do the standard open-source tactic of supporting old releases with bug fixes while still moving into the future. People are always free to install the old version. 😃 Does that seem reasonable?

@JonnyJD
Copy link
Collaborator

JonnyJD commented Jun 4, 2015

I agree with @sampsyo. Now is the time to use requests. Everyone that bothers with a new musicbrainzngs can also get requests easily.
Normally I would suggest raising the major version for an "apichange" (depending on definition) like this, but since our major version is still 0 (still not stable according to semver.org) raising minor and adding a note in the Changelog seems to be enough.

@mineo
Copy link
Collaborator

mineo commented Jun 4, 2016

I'll close this in favor of #199.

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

Successfully merging this pull request may close these issues.

5 participants