Skip to content

refactor: move TransportSocketOptions into LoadBalancerContext#8424

Merged
lizan merged 4 commits intoenvoyproxy:masterfrom
lizan:tso_lbcontext
Oct 1, 2019
Merged

refactor: move TransportSocketOptions into LoadBalancerContext#8424
lizan merged 4 commits intoenvoyproxy:masterfrom
lizan:tso_lbcontext

Conversation

@lizan
Copy link
Copy Markdown
Member

@lizan lizan commented Sep 28, 2019

Description:
Needed for #2690 to use TransportSocketOptions in HTTP cluster.

Risk Level: Low (refactor only)
Testing: existing
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan requested a review from zuercher as a code owner September 28, 2019 22:54
Copy link
Copy Markdown
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple nits

virtual Network::Socket::OptionsSharedPtr upstreamSocketOptions() const PURE;

/**
* Returns the transprot socket options which should be applied on upstream connections
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.

typo: transport

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Weird that the spell checker didn't find this.

if (transport_socket_options != nullptr) {
transport_socket_options->hashKey(hash_key);
bool have_transport_socket_options = false;
if (context && context->upstreamTransportSocketOptions() != nullptr) {
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.

nit: context != nullptr

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan requested a review from snowp October 1, 2019 00:06
Copy link
Copy Markdown
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

One nit, but otherwise LGTM.

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Copy link
Copy Markdown
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

LGTM

@lizan lizan merged commit a662ad5 into envoyproxy:master Oct 1, 2019
@lizan lizan deleted the tso_lbcontext branch October 1, 2019 21:13
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
…proxy#8424)

Description:
Needed for envoyproxy#2690 to use TransportSocketOptions in HTTP cluster.

Risk Level: Low (refactor only)
Testing: existing
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
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