mapbox / mapbox-sdk-py

Python SDK for Mapbox APIs **DEVELOPMENT IS TEMPORARILY PAUSED, SEE CONTRIBUTING.md**
MIT License
320 stars 112 forks source link

Analytics API #153

Closed perrygeo closed 7 years ago

perrygeo commented 7 years ago

Still in preview and available only for premium and enterprise plans: https://www.mapbox.com/api-documentation/#analytics

amishas157 commented 7 years ago

As per discussion w/ @perrygeo and @batpad , I would like to take work on sdk-py for Analytics API ahead. Based on reading through existing sdk-py, found the workflow as following:

Analytics API provides usage statistics for different services provided by Mapbox. It receives parameters as following:

Note : A secret access token is required which has a scope of analytics::read to create the session.

Response: This returns timestamps, period and different services in form of JSON. Services and usage report depend on the resource type one request for.

I have a general query that if the input parameters need to be validated at SDK end as well, in case when they are getting validated at API level? For example, As I mentioned Period validation above, if we give wrong parameter to API, the response will contain error which we can directly pass to the client back. Or what are the right practices?

@perrygeo @batpad Let me know if I missed out on something or misunderstood something. Would be glad to hear your thoughts on above.

batpad commented 7 years ago

I have a general query that if the input parameters need to be validated at SDK end as well, in case when they are getting validated at API level?

From looking at the other services, @amishas157, it seems like we would want to do basic validation in the SDK before sending the request to the API. I'd follow a similar pattern to the other services, eg: https://github.com/mapbox/mapbox-sdk-py/blob/master/mapbox/services/directions.py#L21

There does seem to be a catch-all for errors returned from the back-end service: https://github.com/mapbox/mapbox-sdk-py/blob/master/mapbox/services/base.py#L68, but from my understanding, it should only fallback to this if there is some problem with the service (eg. a service outage), OR if there are parameters that we have no way of validating on the client-side (i.e. it requires some checking with a back-end data-store or so, for eg. whether a supplied valid looking token is actually valid). For basic input validation of formats and data-types, this should be handled by the SDK before making the request.

Hope my understanding here was correct.

It's possible it might be helpful to write a README first to outline how you see the interface working in the SDK: https://github.com/mapbox/mapbox-sdk-py/tree/master/docs and make sure everyone is on the same page.

perrygeo commented 7 years ago

@amishas157 Fantastic! Thanks for the summary, we're on the right track.

I have a general query that if the input parameters need to be validated at SDK end as well, in case when they are getting validated at API level?

do basic validation in the SDK before sending the request to the API.

👍 There was some discussion early on about where the validation should happen (CLI, SDK or just rely on the API response). The conclusion was that the SDK should be responsible for basic validation of parameters wherever possible; in other words the SDK should catch any obvious input errors and raise an appropriate exception before hitting the API.

A secret access token is required which has a scope of analytics::read to create the session

Errors such as passing a token without the proper scope cannot be handled on the SDK side. For that, we make the request and check the status code of the response. The upload error handling provides an example.

it might be helpful to write a README first

💯 That's a great idea. The markdown examples in our /docs directory are written in Python doctest style (notably with the preceding >>>) in order to automatically test them. We don't necessarily need to do that immediately but it might be helpful if you work well with test-driven development.

For example, to test the static API docs:

$ py.test --doctest-glob='*.md' docs/static.md
============================================== test session starts ==============================================
platform darwin -- Python 3.5.2, pytest-3.0.2, py-1.4.31, pluggy-0.3.1
rootdir: /Users/mperry/work/mapbox-sdk-py, inifile: pytest.ini
plugins: xdist-1.15.0, mypy-0.2.0, ipdb-0.1.dev2, flake8-0.8.1, cov-2.3.1, hypothesis-3.5.2
collected 1 items

docs/static.md .

=========================================== 1 passed in 2.86 seconds ============================================

These tests (unlike our unit tests) hit the live API endpoint and thus provide a pretty good real world integration test to demonstrate code usage.

sgillies commented 7 years ago

@amishas157 very grateful for your help 😃

amishas157 commented 7 years ago

Thanks a lot @batpad @perrygeo @sgillies . This was really heplful.

It's possible it might be helpful to write a README first to outline how you see the interface working in the SDK: https://github.com/mapbox/mapbox-sdk-py/tree/master/docs and make sure everyone is on the same page.

Seems like a great idea. Coming with it soon.

perrygeo commented 7 years ago

@amishas157 and @sgillies finished this work in #161 🎉