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

Investigate sending parameters as json encoded body vs. query string in SDK #42

Open
baumatron opened this issue Jan 2, 2017 · 3 comments

Comments

@baumatron
Copy link
Contributor

It seems odd that, for example, the event recording portion of the SDK submits events using json, but the analysis requests pack the request parameters into a url-encoded parameter list as part of the request url. Really it makes sense that requests could be done either way, but mixing both ways in the SDK seems unnecessary and probably just adds code and complexity to the SDK.

This usage might have an impact on caching which would be good to consider.

Keen IO API documentation seems to suggest an application/json request body should be used for analyses and doing requests in this way might simplify how requests are done in general across the SDK.

Code such as Queries.KeenWebApiRequest and the methods on Queries that use it could be simplified to make use of Newtonsoft.Json to build the json request body. Going further, code that builds a request across the SDK could be consolidated. There are currently 6 places in the SDK that each create an instance of HttpClient, set authorization headers, make a request, and get the result. I bet there's an opportunity there to get rid of code by creating a common code path. Additionally, guidance for usage of HttpClient is to reuse instances.

@masojus
Copy link
Contributor

masojus commented Apr 14, 2017

I'm sure the theory here originally was that GET is more REST-ful for a query, but we should move to POST so we can have bigger payloads. Locally I've unified the HTTP message dispatching, and then we can later move to generating the payload for queries such that they can be used with Post*() easily, roughly similar to how we handle this in the Java SDK. This would fix Issue #32 of course.

Caching is a good point...we'll have to chat about that.

@masojus
Copy link
Contributor

masojus commented Apr 29, 2017

PHP analogue: keenlabs/KeenClient-PHP#111

@masojus
Copy link
Contributor

masojus commented Jul 18, 2017

Python is headed that way soon, too: keenlabs/KeenClient-Python#130

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

No branches or pull requests

2 participants