Skip to content

Remove parameterization from TcpTransport#27407

Merged
Tim-Brooks merged 6 commits intoelastic:masterfrom
Tim-Brooks:remove_parameter_from_tcp_transport
Nov 16, 2017
Merged

Remove parameterization from TcpTransport#27407
Tim-Brooks merged 6 commits intoelastic:masterfrom
Tim-Brooks:remove_parameter_from_tcp_transport

Conversation

@Tim-Brooks
Copy link
Copy Markdown
Contributor

This commit is a follow up to the work completed in #27132. Essentially
it transitions two more methods (sendMessage and getLocalAddress) from
TcpTransport to TcpChannel. With this change, there is no longer a need for
TcpTransport to be aware of the specific type of channel a transport
returns. So that class is no longer parameterized by channel type.

This commit is a follow up to the work completed in elastic#27132. Essentially
it transitions two more methods (sendMessage and getLocalAddress) from
Transport to TcpChannel. With this change, there is no longer a need for
TcpTransport to be aware of the specific type of channel a transport
returns. So that class is no longer parameterized by channel type.
@Tim-Brooks Tim-Brooks added :Distributed/Network Http and internode communication implementations >non-issue review v6.1.0 v7.0.0 labels Nov 15, 2017
@Tim-Brooks
Copy link
Copy Markdown
Contributor Author

As a note, I have realized that it does not make sense for NioChannel to extend TcpChannel. NioChannel is selectable channel that can be handled by nio. TcpChannel is a channel that provides the functionality for our tcp binary protocol. We eventually will have some http channel that is an NioChannel, but not a TcpChannel. Right now this causes some issues for parameterized listener types in the nio transport. Once this is merged, I will follow this PR up with another PR that fixes this issue.

Copy link
Copy Markdown
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

so much goodness! LGTM

}

@Override
public void sendMessage(BytesReference reference, ActionListener<TcpChannel> listener) {
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.

can we have a more verbose TODO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I'm not sure what happened to the rest of the comment.


@Override
public void sendMessage(BytesReference reference, ActionListener<TcpChannel> listener) {
// TODO: temporar
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.

can't you just use the passed in listener?

Copy link
Copy Markdown
Contributor Author

@Tim-Brooks Tim-Brooks Nov 16, 2017

Choose a reason for hiding this comment

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

No. ActionListener<TcpChannel> != ActionListener<NioChannel> to the compiler. I will fix this in a follow up (#27407 (comment)).

@Tim-Brooks Tim-Brooks merged commit 80ef9bb into elastic:master Nov 16, 2017
Tim-Brooks added a commit that referenced this pull request Nov 16, 2017
This commit is a follow up to the work completed in #27132. Essentially
it transitions two more methods (sendMessage and getLocalAddress) from
Transport to TcpChannel. With this change, there is no longer a need for
TcpTransport to be aware of the specific type of channel a transport
returns. So that class is no longer parameterized by channel type.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this pull request Nov 17, 2017
This is a followup to elastic#27407. That commit removed the channel type
parameter from TcpTransport. This commit removes the parameter from the
handshake response handler.
Tim-Brooks added a commit that referenced this pull request Nov 17, 2017
This is a followup to #27407. That commit removed the channel type
parameter from TcpTransport. This commit removes the parameter from the
handshake response handler.
Tim-Brooks added a commit that referenced this pull request Nov 17, 2017
This is a followup to #27407. That commit removed the channel type
parameter from TcpTransport. This commit removes the parameter from the
handshake response handler.
@Tim-Brooks Tim-Brooks deleted the remove_parameter_from_tcp_transport branch December 10, 2018 16:19
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Network Http and internode communication implementations >non-issue v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants