Make discogs_client python 2 & python 3 compatible #43

brunal · 2015-01-16T17:09:41Z

This PR makes discogs_client python 2 and python 3 compatible.

Notables changes include:

switch from python-oauth2 (no py3k compatibility) to oauthlib (→ helps with Non-ASCII searches fail because python-oauth2 is broken #40)
fix numerous pep8 error
make tests deterministic

provide a "if __main__ == '__main__'" at the bottom of each test module so they can be executed by themselves: $ python discogs_client/tests/test_module.py [optional Class.test_name]

Python 2 & 3 compatibility. Memory usage may slightly increase in python 2.

Introduce a new dependency: six

oauth2 package is python2-only while oauthlib is compatible with python 2 & 3.

parse_qs() result is non-detereministic → a different file might be requested, making the tests fail randomnly. This commit brings symlinks for all potential targets of the tests, thus fixing them.

It is required by the OAuth spec and it was missing.

coveralls · 2015-01-16T17:14:09Z

Coverage decreased (-0.49%) when pulling 3f43257 on brunal:py23 into 7a97275 on discogs:master.

That's one dirty solution. The real fix would be dropping support for python 2.6 though.

coveralls · 2015-01-16T17:32:21Z

Coverage decreased (-0.66%) when pulling edef8e3 on brunal:py23 into 7a97275 on discogs:master.

rodneykeeling · 2015-01-16T18:53:40Z

Nice! Thanks for the update, @brunal! 👍 from me.

rodneykeeling · 2015-01-16T19:14:37Z

@brunal looks like there's an issue with Python 3:

>>> import discogs_client
>>> d = discogs_client.Client('Test/0.1')
>>> d.release(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/foobar/Projects/discogs_client/discogs_client/models.py", line 490, in __repr__
    return '<Release %r %r>' % (self.id, self.title)
  File "/home/foobar/Projects/discogs_client/discogs_client/models.py", line 30, in __get__
    value = instance.fetch(self.name)
  File "/home/foobar/Projects/discogs_client/discogs_client/models.py", line 241, in fetch
    self.refresh()
  File "/home/foobar/Projects/discogs_client/discogs_client/models.py", line 207, in refresh
    data = self.client._get(self.data['resource_url'])
  File "/home/foobar/Projects/discogs_client/discogs_client/client.py", line 117, in _get
    return self._request('GET', url)
  File "/home/foobar/Projects/discogs_client/discogs_client/client.py", line 109, in _request
    body = json.loads(content)
  File "/usr/lib64/python3.4/json/__init__.py", line 312, in loads
    s.__class__.__name__))
TypeError: the JSON object must be str, not 'bytes'

Could you update .travis.yml to test for Python 3 as well?

utf8-decoding the (json) response happens right before parsing it. Adapt several fetch() implementations, update test data and improve fetch() docstring.

coveralls · 2015-01-17T11:33:23Z

Coverage decreased (-0.01%) when pulling 0c52119 on brunal:py23 into 7a97275 on discogs:master.

brunal · 2015-01-17T12:05:15Z

That issue is now fixed! I hoped there's not more of them lying hidden. I added python 3.{2,3,4} on travis.

I took the decision of making Client.fetch() return bytes, later decoded over Client.fetch() returning a string since the most important Client scions use requests.request, which returns bytes.

rodneykeeling · 2015-01-21T18:31:49Z

Hey @brunal,

I just tried walking through the OAuth process on the README (using Python 2), but I get an error on the d.get_access_token('my_token') step. Can you look into that?

Thanks,
Rodney

brunal · 2015-01-21T18:57:37Z

Done!

coveralls · 2015-01-21T19:09:00Z

Coverage decreased (-1.56%) to 84.57% when pulling ff0a5d0 on brunal:py23 into 7a97275 on discogs:master.

coveralls · 2015-03-02T11:34:16Z

Coverage decreased (-1.49%) to 84.64% when pulling 02ec1f9 on brunal:py23 into 7a97275 on discogs:master.

brunal · 2015-03-10T10:54:59Z

Hi,
I've been using my version with success for the past month. It solves other bugs, such as a 401 error on empty searches.

Do you plan on merging it? Otherwise we might have to use my fork for beets

rodneykeeling · 2015-03-10T16:14:43Z

Thanks @brunal I'll look over this one today!

rodneykeeling · 2015-03-10T16:52:49Z

Following the steps on the README using Python 2.7.6, I get this error when trying to execute me.wantlist.add(ds.release(5)):

ValueError: Body contains parameters but Content-Type header was not set.

The release was successfully added to my wantlist, but this error does not show on master.

Even if it's json use application/x-www-form-urlencoded and never application/json.

Data shall only contain strings as per oauthlib request.

coveralls · 2015-03-11T11:02:33Z

Coverage decreased (-1.49%) to 84.64% when pulling 05cd2e4 on brunal:py23 into 7a97275 on discogs:master.

brunal · 2015-03-11T11:03:59Z

The error message was actually incorrect: the Content-type was application/json instead of application/x-www-url-formencoded. I fixed it (as well as realease_id that should be a string and not an int for the request to be signed).

rodneykeeling · 2015-03-11T18:32:50Z

Great, looks good so far! Although now you must pass get_access_token() a unicode string (e.g., get_access_token(u'abc123')) or you get a warning message. Is that necessary?

brunal · 2015-03-13T10:23:10Z

That was not necessary, but preferable in case the verifier contains non-ascii characters - which is not the case. I removed it!

coveralls · 2015-03-13T10:29:26Z

Coverage decreased (-1.4%) to 84.73% when pulling 43c4a3f on brunal:py23 into 7a97275 on discogs:master.

rodneykeeling · 2015-03-17T16:56:55Z

@brunal Perfect! Unless any of the other Discogs devs find issues within the week, I'll merge this on Friday. I hope that's alright. Thanks again for the great work on this! 🍻

Make discogs_client python 2 & python 3 compatible

brunal · 2015-03-25T12:33:51Z

Cool :-) It's been my pleasure.
Don't you think it would be time to tag a new version (the current one being v2.0.2)? That'd allow us folks at beets to force it.

brunal added 12 commits January 16, 2015 16:56

PEP8-compliance (E501 ignored)

a2f0663

Allow test modules to run standalone

ae31bba

provide a "if __main__ == '__main__'" at the bottom of each test module so they can be executed by themselves: $ python discogs_client/tests/test_module.py [optional Class.test_name]

Fix utils.update_qs test

4ee0893

Use print as a function

50909c1

Make urllib import python 2 & 3 compatible

70c97be

Use range(), items() and 'except X as x'

130a763

Python 2 & 3 compatibility. Memory usage may slightly increase in python 2.

Use the main __future__ imports in all modules

14fe046

Make Metaclass usage python 2 & 3 compatible

a46f780

Introduce a new dependency: six

Replace Oauth2 with Oauthlib

c56f61a

oauth2 package is python2-only while oauthlib is compatible with python 2 & 3.

Fix tests: symlinks for simulated fetchs

e861b3a

parse_qs() result is non-detereministic → a different file might be requested, making the tests fail randomnly. This commit brings symlinks for all potential targets of the tests, thus fixing them.

Make setup.py executable

5d53015

Client.get_authorize_url sets the Content-Type

3f43257

It is required by the OAuth spec and it was missing.

Fix tests for python 2.6

edef8e3

That's one dirty solution. The real fix would be dropping support for python 2.6 though.

brunal added 3 commits January 17, 2015 11:57

Travis: add support for python 3

4288d49

Update "make clean" for python 3 support

2b6bb52

Client.fetch() always returns bytes

0c52119

utf8-decoding the (json) response happens right before parsing it. Adapt several fetch() implementations, update test data and improve fetch() docstring.

Improve bytes handling

ff0a5d0

Fix unicode mgmt in utils.update_qs()

02ec1f9

brunal mentioned this pull request Mar 10, 2015

Discogs crashes beets 1.3.11 beetbox/beets#1345

Closed

brunal added 2 commits March 11, 2015 11:44

Fix Content-Type of signed requests with body

d75af2a

Even if it's json use application/x-www-form-urlencoded and never application/json.

Wantlist: send release as a string

05cd2e4

Data shall only contain strings as per oauthlib request.

get_access_token(): don't warn on bytes verifier

43c4a3f

rodneykeeling added a commit that referenced this pull request Mar 20, 2015

Merge pull request #43 from brunal/py23

956f82d

Make discogs_client python 2 & python 3 compatible

rodneykeeling merged commit 956f82d into discogs:master Mar 20, 2015

brunal mentioned this pull request Apr 1, 2015

Put a new tag on master #45

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make discogs_client python 2 & python 3 compatible #43

Make discogs_client python 2 & python 3 compatible #43

brunal commented Jan 16, 2015

coveralls commented Jan 16, 2015

coveralls commented Jan 16, 2015

rodneykeeling commented Jan 16, 2015

rodneykeeling commented Jan 16, 2015

coveralls commented Jan 17, 2015

brunal commented Jan 17, 2015

rodneykeeling commented Jan 21, 2015

brunal commented Jan 21, 2015

coveralls commented Jan 21, 2015

coveralls commented Mar 2, 2015

brunal commented Mar 10, 2015

rodneykeeling commented Mar 10, 2015

rodneykeeling commented Mar 10, 2015

coveralls commented Mar 11, 2015

brunal commented Mar 11, 2015

rodneykeeling commented Mar 11, 2015

brunal commented Mar 13, 2015

coveralls commented Mar 13, 2015

rodneykeeling commented Mar 17, 2015

brunal commented Mar 25, 2015

Make discogs_client python 2 & python 3 compatible #43

Make discogs_client python 2 & python 3 compatible #43

Conversation

brunal commented Jan 16, 2015

coveralls commented Jan 16, 2015

coveralls commented Jan 16, 2015

rodneykeeling commented Jan 16, 2015

rodneykeeling commented Jan 16, 2015

coveralls commented Jan 17, 2015

brunal commented Jan 17, 2015

rodneykeeling commented Jan 21, 2015

brunal commented Jan 21, 2015

coveralls commented Jan 21, 2015

coveralls commented Mar 2, 2015

brunal commented Mar 10, 2015

rodneykeeling commented Mar 10, 2015

rodneykeeling commented Mar 10, 2015

coveralls commented Mar 11, 2015

brunal commented Mar 11, 2015

rodneykeeling commented Mar 11, 2015

brunal commented Mar 13, 2015

coveralls commented Mar 13, 2015

rodneykeeling commented Mar 17, 2015

brunal commented Mar 25, 2015