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

Allow uploading in-memory files #356

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe
cc @stripe/api-libraries @praboud-stripe

Fixes #227.

I spent some time investigating this and I agree with @praboud-stripe's analysis in #227. Rack returns multipart blocks with no filenames directly as Strings rather than as Hashes -- they explicitly test for this behavior (cf. here).

Fixing the backend to handle either a string or a hash might be possible, but sending a dummy filename in the client libraries is definitely the path of least resistance. We actually already do this in stripe-node (cf. here).

@@ -7,7 +8,7 @@ class FileUpload(ListableAPIResource):

@classmethod
def api_base(cls):
return upload_api_base
return stripe.upload_api_base
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opportunistic fix to ensure that the current value of upload_api_base is used (before, the value was only read once when the library is loaded).

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find.

@brandur-stripe
Copy link
Contributor

Oh man, the conversation we had about this in #227 was like 10x longer than the fix turned out to be! Thanks for being the one to take action here Olivier :) And thanks for adding the new test too — really helpful in helping to understand that this does the right thing.

LGTM.

@brandur-stripe brandur-stripe merged commit 1506f4e into master Oct 23, 2017
@brandur-stripe brandur-stripe deleted the ob-file-upload-stringio branch October 23, 2017 14:37
@brandur-stripe
Copy link
Contributor

Released as 1.70.0.

@praboud-stripe
Copy link

Thanks @ob-stripe !

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