Add multipart file upload to PostAgent #1690
Conversation
| @@ -113,6 +113,7 @@ def faraday | |||
| unless boolify(interpolated['disable_redirect_follow']) | |||
| builder.use FaradayMiddleware::FollowRedirects | |||
| end | |||
| builder.request :multipart | |||
There was a problem hiding this comment.
Will this affect existing users even if they're not uploading files?
There was a problem hiding this comment.
It shouldn't the middleware only is called when one of the parameter responds to content_type.
| @@ -162,22 +171,29 @@ def normalize_response_headers(headers) | |||
| } | |||
| end | |||
|
|
|||
| def handle(data, payload = {}, headers) | |||
| url = interpolated(payload)[:post_url] | |||
| def handle(data, event = Event.new, headers) | |||
There was a problem hiding this comment.
Sidenote: it's odd that we have a middle parameter with a default. I wonder if we ever use that?
There was a problem hiding this comment.
True, I guess the headers parameter was added as a third when the Agent got support for specifying the headers. check is calling handle with only two parameters.
|
|
||
| case method | ||
| when 'get', 'delete' | ||
| params, body = data, nil | ||
| when 'post', 'put', 'patch' | ||
| params = nil | ||
|
|
||
| case (content_type = interpolated(payload)['content_type']) | ||
| content_type = if has_file_pointer?(event) |
There was a problem hiding this comment.
I think this would be slightly more readable as:
if has_file_pointer?(event)
data[interpolated(event.payload)['upload_key'].presence || 'file'] = get_upload_io(event)
content_type = nil
else
content_type = interpolated(event.payload)['content_type']
endThere was a problem hiding this comment.
It's the complete opposite for me, I don't like the assignment to be "hidden" in the conditional :)
The bbatsov style guide suggest:
content_type =
if has_file_pointer?(event)
data[interpolated(event.payload)['upload_key'].presence || 'file'] = get_upload_io(event)
nil
else
interpolated(event.payload)['content_type']
endThere was a problem hiding this comment.
Hah. I don't feel strongly, go with whichever you prefer. Personally, I find it confusing that the if has both a side effect (assigning into data) and is also being used for assignment itself. The new one is fine too.
|
Nice! |
|
LGTM :) |
This allows the PostAgent to receive `file_pointer` events and upload the data to a website.
dsander
force-pushed
the
post-agent-file-upload
branch
from
c744ddf
to
663eebc
Compare
|
Thanks 😄 |
This allows the PostAgent to receive
file_pointerevents and upload the data to a website.