yola / demands Goto Github PK

pip seems to pull the OS X binary first in some environments: https://pypi.python.org/pypi/demands

https://gumroad.com/l/RRZc

We need to send client name and app name to API hosts.

If we implement an API client for a service named BOB, BOB should know, for each request, the name and version of our client, as well as the name of the thing using our client. Offhand, the User-Agent header seems to me like a good candidate for hijacking. Sending something like clientname x.y.z - appname would be great. We'd just need some flags somewhere for these strings and we're golden.

This came about because of a conversation with @blaix. Perhaps he has additional input.

Then it can be used as a dependency in other open source projects

When page_size of the PaginatedResults is greater than page size of requested endpoint, then paginator does not return all data.
Example (page size of endpoint in list_cname_zones = 10):

>>> len(list(PaginatedResults(DomainService().list_cname_zones, user_id=user_id, page_size=1)))
27
>>> len(list(PaginatedResults(DomainService().list_cname_zones, user_id=user_id, page_size=10)))
27
>>> len(list(PaginatedResults(DomainService().list_cname_zones, user_id=user_id, page_size=100)))
10

A new version of DRF responds with 404 for an empty page (when page number > 1). As result, we might end up with an error in our code when trying to iterate over pagination results. E.g. for the cases when a number of resources is a multiple of page size (e.g. page_size = 10 and number of results is 20).

Proposed solution: when we iterate over results - no need to request next page, if next meta key is null (indicator that the next page is empty).

It might be nice to have demands throw a 502 when an upstream server 500s. Right now demands will throw a 500 when an upstream server 500s. When I get a 500 back my immediate assumption is that the server I am talking with directly experienced an error, not an upstream server. It's minor, but I think the whole point of demands is to make it easier for developers, and I think this could help. Thoughts?

We use getargspec here, and in Python 3.5 it's started raising deprecation warnings when running the tests.

inspect.signature() docs | inspect.getargspec() docs

Django PR for dealing with it: https://github.com/django/django/pull/4846/files

Technically it's not an iterator, but rather a container which supports iteration. And it's a bit misleading (I was wondering why I couldn't do next() over its instance).

The question here, should we rename it, to something less misleading (e.g. PaginatedAPIResults or anything better) or the current name is fine?

ping @yola/engineers

Our HTTPServiceClient derives from requests.Session and, as a result, its default behaviour is to share some params, cookies and connection between requests (http://docs.python-requests.org/en/latest/user/advanced/#session-objects). The default behaviour of requests.get/post/put/delete is to create a new Session for each request. Taking into account that we usually cache our demands based clients on class level and almost never close the session (but requests recommend to do that https://github.com/kennethreitz/requests/blob/master/requests/api.py#L49-L51) - I'm concerned that it might be the reason for different bugs/issues etc.

What if we change our base client default behaviour to create a new Session (connection) for each request? But also leave the possibility to share session between requests (just, for example, when we initialize it in a special way). So, the same client could work in 2 modes: creating a new session for each request (default) or using shared session (as a context manager).

If my concerns are groundless we can just close this issue and continue to use our demands as we were before.

Thoughts?

ping @blaix, @thomasw, @stefanor, @ekoval

This pagination type should probably require wrapped client's method to accept and handle next_page_url.

Related discussion: #70 (comment)

Looks like we should just remove some interpreters (and probably add Python 3.5)

I end up copy pasta'ing this mock or a similar one every time I write tests for a demands client.

I really like how we deal with pagination in https://github.com/yola/pycloudflare. I think we should move it here to be able to re-use it in other clients.

Newer requests accept additional json= param, which allows us to make JSON-calls e.g.
post(<SOME_URL>, json={...}) will actually do what our JSONServiceClient does.

Taking into account the above, I think we could remove JSONServiceClient at all. Since it doesn't look very useful now.

What's your opinion?