Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

Consider getting rid of the client CONVENIENCE_HEADERS handling #63

Open
jbandhauer opened this issue Jul 12, 2017 · 0 comments
Open

Consider getting rid of the client CONVENIENCE_HEADERS handling #63

jbandhauer opened this issue Jul 12, 2017 · 0 comments

Comments

@jbandhauer
Copy link
Contributor

The client code inherited a scheme for plucking out query headers that seem like they are intended to be in the headers.

That is done in parse_query_and_convenience_headers at
https://github.com/looker/looker-sdk-ruby/blob/master/lib/looker-sdk/client.rb#L458-L462

The specific headers it supports are:
CONVENIENCE_HEADERS = Set.new([:accept, :content_type])

It isn't clear that this is doing anyone any good. And, there have been a couple known instances of this being a pain for people.

Because Looker API designers were not precluded from using these strings as query param names, there are some endpoints in Looker that use content_type as a param. Those endpoints are thus confusingly difficult to use from the SDK.

The workaround is to explicitly declare the query parts like:

Instead of:

sdk.foo({content_type: 'bar', baz: 1})

Use:

sdk.foo({query: {content_type: 'bar', baz: 1}})

The SDK allows for explicitly saying which params are for the query part and which are intended to be headers. So, one can still do:

sdk.foo({query: {content_type: 'bar', baz: 1}, headers:{content_type: 'json'}})

Nevertheless, my proposal is to simply remove the CONVENIENCE_HEADERS stuff so that the only way to have hash members end up in the headers is to do it explicitly.

Yet another case of the 'helpful' name-specific handling getting in our way more than helping us.

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

No branches or pull requests

1 participant