Skip to content

netty: move sslContext into transportCreationParamsFilterFactory - #3111

Merged
ericgribkoff merged 1 commit into
grpc:masterfrom
ericgribkoff:netty_ssl_context
Jun 20, 2017
Merged

ericgribkoff merged 1 commit into
grpc:masterfrom
ericgribkoff:netty_ssl_context

Conversation

@ericgribkoff

Copy link
Copy Markdown
Contributor

Eagerly initializing sslContext (#3060) can throw unnecessarily when InternalNettyChannelBuilder#setDynamicTransportParamsFactory has been used to set a transport params factory. This PR introduces DefaultNettyTransportCreationParamsFilterFactory, and moves the final sslContext member variable from NettyTransportFactory into DefaultNettyTransportCreationParamsFilterFactory. This also moves the existing nested DynamicNettyTransportParams class into DefaultNettyTransportCreationParamsFilterFactory.
Unfortunately, the git diff here is not very helpful with seeing the code movement.

I think this is a reasonable way to re-arrange things: we only use sslContext if the transportCreationParamsFilterFactory parameter to the NettyTransportFactory is null, in which case we create a new TransportCreationParamsFilterFactory, so it makes sense to move sslContext into this new class.

@carl-mastrangelo carl-mastrangelo left a comment

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.

LGTM

@ericgribkoff
ericgribkoff merged commit e01cec5 into grpc:master Jun 20, 2017
@ericgribkoff
ericgribkoff deleted the netty_ssl_context branch June 20, 2017 06:21
@lock lock Bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants