-
Notifications
You must be signed in to change notification settings - Fork 227
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
Support for binary response and binary request body in java8 template #159
Comments
Hi @erlendv, Thank you for your suggestion and for providing lots of detail. I could see this as being a useful change for Java users. I can see that you've followed some of the contribution guide, but there is a part that states that work shouldn't be started until the feature is agreed on with the maintainers. Given that you've already done the work, let's chat about it here? Is this a breaking change? Alex |
Hi Alex
Thank you for your reply. Sorry about that, I realise I did not quite
follow the contribution guide here. I got a bit eager :)
Im new to openfass and not sure how this compares to the other language
support. But I think being able to read and write binary data could be a
good thing. For instance an image manipulation service.
Ideally I would have solved this a bit different. But the goal was not to
break anything, so no, its not a breaking change. But I think this is a
good compromise.
I added some tests also, they should verify that it doesnt break, as well
as test the new functionality.
Best regards
Erlend
fre. 19. jul. 2019 kl. 21:52 skrev Alex Ellis <[email protected]>:
… Hi @erlendv <https://github.com/erlendv>,
Thank you for your suggestion and for providing lots of detail. I could
see this as being a useful change for Java users.
I can see that you've followed some of the contribution guide, but there
is a part that states that work shouldn't be started until the feature is
agreed on with the maintainers.
Given that you've already done the work, let's chat about it here?
Is this a breaking change?
Alex
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#159?email_source=notifications&email_token=AAKSDYN5SU4GQPAMU2HDEKTQAILRLA5CNFSM4IFD6AN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2MTHCI#issuecomment-513356681>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKSDYNTNAPE5UMJRVH3R4LQAILRLANCNFSM4IFD6ANQ>
.
|
Hi Alex
This is my first contribution to anything so I hope you can bear with me. I did forget to sign off the first commit and tried to amend the commit with signoff. But the result was two commits:
ca71c0e <ca71c0e>
d843d6f <d843d6f>
I have pushed the changes you proposed.
Best regards,
Erlend
… 19. jul. 2019 kl. 21:52 skrev Alex Ellis ***@***.***>:
Hi @erlendv <https://github.com/erlendv>,
Thank you for your suggestion and for providing lots of detail. I could see this as being a useful change for Java users.
I can see that you've followed some of the contribution guide, but there is a part that states that work shouldn't be started until the feature is agreed on with the maintainers.
Given that you've already done the work, let's chat about it here?
Is this a breaking change?
Alex
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#159?email_source=notifications&email_token=AAKSDYN5SU4GQPAMU2HDEKTQAILRLA5CNFSM4IFD6AN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2MTHCI#issuecomment-513356681>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKSDYNTNAPE5UMJRVH3R4LQAILRLANCNFSM4IFD6ANQ>.
|
Any news on this? Cant be true that I can only exchange strings in functions right? Base64 encoding an image adds a substantial overhead I would like to avoid... |
The IResponse interface only have a setBody()/getBody() that works with strings. The entrypoint (com.openfaas.entrypoint.App) also tries to encode the body to UTF-8 when converting to bytes to write to the OutputStream.
Also, the IRequest interface also only have a getBody() that returns a String.
Expected Behaviour
The IResponse-interface should at least have a setBody(byte[] data) and a getBodyData() (that gets the byte). The entrypoint should use getBodyData() instead of getBody().
The best solution would be to write to an OutputStream on the response, ideally being the OutputStream from HttpExchange. getResponseBody().
The IRequest interface should have a getInputStream() method to return an InputStream, and the Request-implementation should return the one from HttpExchange.getRequestBody()
Current Behaviour
Unable to write binary data.
Possible Solution
See expected behaviour.
Steps to Reproduce (for bugs)
Context
In my case I was serializing java objects between the function and a java client. Controlling both I base64-encoded the data to work around this. However if the response where to be an image for example it should be possible to return raw data.
Your Environment
The text was updated successfully, but these errors were encountered: