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

Content-Type header #3

Merged
merged 6 commits into from
May 16, 2019
Merged

Content-Type header #3

merged 6 commits into from
May 16, 2019

Conversation

kipters
Copy link
Contributor

@kipters kipters commented Apr 16, 2019

This PR adds a method to the runner builder so the user can specify a value for the Content-Type header, that gets injected in responses when hosting functions that return a result.

It uses "application/json" as a default in case nothing is specified. This may be considered a breaking change, but I think it's a sane default nonetheless.

Copy link
Contributor

@Kralizek Kralizek left a comment

Choose a reason for hiding this comment

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

Great idea. I definitely agree that we should not have a major version bump.

I would just look at the sequence. It doens't make sense to specify the response content type if there is no response. The fluent builder should not give the user the possibility to specify options that are at best useless.

Also I would consider the possibility to have the serializer specify the content type, although this would mean moving away from the ILambdaSerializer interface.

Also, please update the README with an example.

@Kralizek Kralizek merged commit 459b577 into keghub:master May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants