Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull channel creation out of ConnectionManager #1158

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Apr 7, 2021

Motivation:

The ConnectionManager is initialized with a
ClientConnection.Configuration struct because it needs enough "stuff"
to make NIO.Channels with. That's not really information it needs to
hold on to, it just needs a way to make channels. Moreover, it limits
the connection manager to using only that configuration object. That's a
bit of a pain if we want to add a connection pool with a different
configuration object.

Modifications:

  • Pull out the channel creation into a ConnectionManagerChannelProvider
    protocol.
  • Move the ClientConnection.Configuration based channel creation into
    an object conforming to the above.

Result:

Looser coupling.

Motivation:

The `ConnectionManager` is initialized with a
`ClientConnection.Configuration` struct because it needs enough "stuff"
to make `NIO.Channel`s with. That's not really information it needs to
hold on to, it just needs a way to make channels. Moreover, it limits
the connection manager to using only that configuration object. That's a
bit of a pain if we want to add a connection pool with a different
configuration object.

Modifications:

- Pull out the channel creation into a `ConnectionManagerChannelProvider`
  protocol.
- Move the `ClientConnection.Configuration` based channel creation into
  an object conforming to the above.

Result:

Looser coupling.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Apr 7, 2021
@glbrntt glbrntt requested a review from Lukasa April 7, 2021 14:35
@glbrntt glbrntt merged commit ab9165e into grpc:main Apr 7, 2021
@glbrntt glbrntt deleted the gb-conn-man-refactor branch April 7, 2021 15:20
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Apr 14, 2021
Motivation:

In grpc#1158 we pulled connection creation of the connection manager into a
channel provider in order to loosen the coupling between the connection
manager and `ClientConnection`. This change further decouples the
`ConnectionManager` from the channel provider pulling out the relevant
configuration into a `DefaultChannelProvider`.

Modifications:

- Refactor `ClientConnection.ChannelProvider` to rely on the bits of
  configuration it actually requires rather than `ClientConnection.Configuration`
- Rename to `DefaultChannelProvider`

Result:

We can configure channels for the `ConnectionManager` without being tied
to `ClientConnection`.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Apr 14, 2021
Motivation:

In grpc#1158 we pulled connection creation of the connection manager into a
channel provider in order to loosen the coupling between the connection
manager and `ClientConnection`. This change further decouples the
`ConnectionManager` from the channel provider pulling out the relevant
configuration into a `DefaultChannelProvider`.

Modifications:

- Refactor `ClientConnection.ChannelProvider` to rely on the bits of
  configuration it actually requires rather than `ClientConnection.Configuration`
- Rename to `DefaultChannelProvider`

Result:

We can configure channels for the `ConnectionManager` without being tied
to `ClientConnection`.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Apr 14, 2021
Motivation:

In grpc#1158 we pulled connection creation of the connection manager into a
channel provider in order to loosen the coupling between the connection
manager and `ClientConnection`. This change further decouples the
`ConnectionManager` from the channel provider pulling out the relevant
configuration into a `DefaultChannelProvider`.

Modifications:

- Refactor `ClientConnection.ChannelProvider` to rely on the bits of
  configuration it actually requires rather than `ClientConnection.Configuration`
- Rename to `DefaultChannelProvider`

Result:

We can configure channels for the `ConnectionManager` without being tied
to `ClientConnection`.
glbrntt added a commit that referenced this pull request Apr 14, 2021
Motivation:

In #1158 we pulled connection creation of the connection manager into a
channel provider in order to loosen the coupling between the connection
manager and `ClientConnection`. This change further decouples the
`ConnectionManager` from the channel provider pulling out the relevant
configuration into a `DefaultChannelProvider`.

Modifications:

- Refactor `ClientConnection.ChannelProvider` to rely on the bits of
  configuration it actually requires rather than `ClientConnection.Configuration`
- Rename to `DefaultChannelProvider`

Result:

We can configure channels for the `ConnectionManager` without being tied
to `ClientConnection`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants