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

Use client to fetch, resolve body and content issues #79

Merged
merged 1 commit into from
Dec 23, 2014
Merged

Use client to fetch, resolve body and content issues #79

merged 1 commit into from
Dec 23, 2014

Conversation

bpicolo
Copy link
Contributor

@bpicolo bpicolo commented Dec 23, 2014

So, the original goal of this branch was just to use the actual client to fetch the api docs, and it now does that (though it doesn't do anything truly asynchronously). But I kind of got sidetracked. Partially fixes #73 but does not number #71.

Turns out a bunch of the tests expected HTTP bodies in GET requests, which shouldn't happen. It was also a bit awkward how we managed content types. So I killed off a lot of body awkwardness and content type awkwardness. Pretty sure this isn't a breaking change unless you're depending on a bad behavior.

Also a couple small refactors for qol.

if not swagger_type.is_primitive(type_):
# If not primitive, body has to be 'dict'
# (or has already been converted to dict from model)
request['headers']['content-type'] = APP_JSON
request['data'] = json.dumps(value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of a key part of this commit. If we claim something is json we should make sure it actually is right there and then.

@dnephin
Copy link
Contributor

dnephin commented Dec 23, 2014

these changes lgtm

bpicolo pushed a commit that referenced this pull request Dec 23, 2014
Use client to fetch, resolve body and content issues
@bpicolo bpicolo merged commit d922522 into Yelp:master Dec 23, 2014
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.

Unify the way we handle api-doc requests with other requests
2 participants