Skip to content

Local transport now uses tcp.#106

Closed
NiteshKant wants to merge 1 commit intorsocket:masterfrom
NiteshKant:local
Closed

Local transport now uses tcp.#106
NiteshKant wants to merge 1 commit intorsocket:masterfrom
NiteshKant:local

Conversation

@NiteshKant
Copy link
Copy Markdown
Contributor

Problem

Local transport was using a custom local socket implementation.
Netty provides a LocalChannel abstraction that can be used.

Modification

Modified reactivesocket-transport-local to use reactivesocket-transport-tcp.
Refactored common test classes to reactivesocket-test module so they can be used in both tcp and local transport without code duplication.

@NiteshKant NiteshKant assigned NiteshKant and stevegury and unassigned NiteshKant Jun 27, 2016
/**
* An implementation of {@link Payload}
*/
public class PayloadImpl implements Payload {
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.

This being available is very helpful

@stevegury
Copy link
Copy Markdown
Member

This looks good, but one of the test is failing.

@NiteshKant
Copy link
Copy Markdown
Contributor Author

@stevegury yes, there is some flakiness in the tests with local channel. I will get back to this tomorrow may be.

@NiteshKant NiteshKant modified the milestones: 0.2.3, 0.2.2 Jun 28, 2016
Problem

Local transport was using a custom local socket implementation.
Netty provides a `LocalChannel` abstraction that can be used.

Modification

Modified reactivesocket-transport-local` to use `reactivesocket-transport-tcp`.
Refactored common test classes to `reactivesocket-test` module so they can be used in both tcp and local transport without code duplication.
NiteshKant added a commit to NiteshKant/reactivesocket-java that referenced this pull request Jul 7, 2016
Problem

This is a split of PR rsocket#106 to make local transport use TCP.
There seems to be some issue in netty's local transport which creates race-conditions and causes test failures.
These refactor has some changes that are not necessarily related to local transport but makes the usage better.

Modification

Refactored common test classes to `reactivesocket-test` module so they can be used in both tcp and local transport without code duplication.
Added `PayloadImpl` as a utility implementation of `Payload`.
@NiteshKant
Copy link
Copy Markdown
Contributor Author

Closing this in favor of #134 which picks all changes from here other than actually changing local transport.
I will start another PR just for local transport changes.

@NiteshKant NiteshKant closed this Jul 7, 2016
NiteshKant added a commit that referenced this pull request Jul 7, 2016
Problem

This is a split of PR #106 to make local transport use TCP.
There seems to be some issue in netty's local transport which creates race-conditions and causes test failures.
These refactor has some changes that are not necessarily related to local transport but makes the usage better.

Modification

Refactored common test classes to `reactivesocket-test` module so they can be used in both tcp and local transport without code duplication.
Added `PayloadImpl` as a utility implementation of `Payload`.
@NiteshKant NiteshKant deleted the local branch July 7, 2016 23:21
NiteshKant added a commit to NiteshKant/reactivesocket-java that referenced this pull request Jul 10, 2016
Problem

This is a split of PR rsocket#106 to make local transport use TCP.
There seems to be some issue in netty's local transport which creates race-conditions and causes test failures.
These refactor has some changes that are not necessarily related to local transport but makes the usage better.

Modification

Refactored common test classes to `reactivesocket-test` module so they can be used in both tcp and local transport without code duplication.
Added `PayloadImpl` as a utility implementation of `Payload`.
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