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

Properly encode file uploads #508

Merged
merged 1 commit into from
Feb 22, 2017
Merged

Conversation

brandur-stripe
Copy link
Contributor

As described in #506, file uploads were broken on the way over to
Faraday and unfortunately I didn't catch it because the file upload
creation test wasn't using a matcher that was strict enough to really
catch anything.

Here we add the multipart middleware to our Faraday connection, add a
compatibility layer to FileUpload so that we can support File like
the rest-client based version always did (Faraday generally expects an
UploadIO object), and then tighten up our tests so that we'd be able
to catch future regressions.

Fixes #506.

r? @ob-stripe Mind taking a look at this one? Thanks!

As described in #506, file uploads were broken on the way over to
Faraday and unfortunately I didn't catch it because the file upload
creation test wasn't using a matcher that was strict enough to really
catch anything.

Here we add the multipart middleware to our Faraday connection, add a
compatibility layer to `FileUpload` so that we can support `File` like
the rest-client based version always did (Faraday generally expects an
`UploadIO` object), and then tighten up our tests so that we'd be able
to catch future regressions.

Fixes #506.
@ob-stripe
Copy link
Contributor

lgtm!

@brandur-stripe
Copy link
Contributor Author

Thanks @ob-stripe!

@brandur-stripe brandur-stripe merged commit 083038e into master Feb 22, 2017
@brandur-stripe brandur-stripe deleted the brandur-fix-file-uploads branch February 22, 2017 16:07
@brandur-stripe
Copy link
Contributor Author

Released as 2.0.1.

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.

3 participants