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

APIGateway fixes #266

Merged
merged 2 commits into from
Jan 3, 2016
Merged

APIGateway fixes #266

merged 2 commits into from
Jan 3, 2016

Conversation

proger
Copy link
Contributor

@proger proger commented Dec 27, 2015

Turns out, APIGateway has two protocol modes, Accept: application/hal+json and Accept: application/json where the latter is the one that's actually specified in the botocore spec.

make -C release MODELS=model/apigateway generates a thorough diff (updated ToHeaders for every request), which is not included.

The only reason I added a whole protocol to gen is to set the header because requestInsts matches on the protocol. It probably makes sense to remove it to make the patch more lightweight, however earlier versions of the patch included a ton of changes for the timestamp format and for pulling the root resource id by parsing stuff in _links before I realised I should try see what botocore does. (。・ω..・)っ So let me know if you'd like the change to be done differently.

- always send "Accept: application/json"
  (this alters the API behavior: it no longer sends application/hal+json
   (which also uses ISO8601 timestamp instead of POSIX)
   and extends its GetResources output to include root resource id:
   https://github.com/boto/botocore/blob/c9e90ef5413bd560422e915d213df73ad88dffd7/tests/integration/test_apigateway.py#L32-L33)

main :: IO ()
main = do
lgr <- newLogger Trace stdout
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hate to bikeshed :trollface: but maybe 4 spaces here, if only because the rest of the project is the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, indeed :)
Pushed again

@brendanhay
Copy link
Owner

👍

@proger
Copy link
Contributor Author

proger commented Jan 3, 2016

Let's merge?

brendanhay added a commit that referenced this pull request Jan 3, 2016
@brendanhay brendanhay merged commit 6cf5462 into brendanhay:develop Jan 3, 2016
@brendanhay
Copy link
Owner

Kone4no!

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

Successfully merging this pull request may close these issues.

2 participants