Skip to content

kotlin: add conversion from Request to headers#296

Merged
buildbreaker merged 4 commits intomasterfrom
ac/request-mapper
Jul 24, 2019
Merged

kotlin: add conversion from Request to headers#296
buildbreaker merged 4 commits intomasterfrom
ac/request-mapper

Conversation

@buildbreaker
Copy link
Copy Markdown

Mirrors: #288

Adds converters to go from Request to a set of headers that upstream Envoy understands.

This will be utilized by the request logic in the Envoy class.

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: kotlin: add conversion from Request to headers
Risk Level: low
Testing: unit
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

Alan Chiu added 4 commits July 23, 2019 17:36
Signed-off-by: Alan Chiu <achiu@lyft.com>
Signed-off-by: Alan Chiu <achiu@lyft.com>
Signed-off-by: Alan Chiu <achiu@lyft.com>
Signed-off-by: Alan Chiu <achiu@lyft.com>
@buildbreaker buildbreaker requested a review from rebello95 July 24, 2019 00:39
}

@Test
fun `options method is added to outbound request headers`() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure these additional tests for each of the RequestMethod cases really provide much value, but no harm in keeping them

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I accidentally messed up writing the mapping from POST to "POST"

I mean the code review would've caught that but a test allowed me to not embarrass myself

@buildbreaker buildbreaker merged commit f59f90c into master Jul 24, 2019
@buildbreaker buildbreaker deleted the ac/request-mapper branch July 24, 2019 01:25
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.

2 participants