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

Add optional service methods #275

Merged
merged 6 commits into from
Jul 10, 2020
Merged

Conversation

hlb8122
Copy link
Contributor

@hlb8122 hlb8122 commented Feb 28, 2020

Motivation

#265

Solution

This is currently an incomplete solution - it is the minimum required to demonstrate this approach and allow for a decision.

It mirrors closely the way optional_layer method works.

Considerations

  • Is this zero-cost?
  • Should NamedService be implemented on Either like this? Does this break anything?
  • There's an Unimplemented service which seems to do the same thing as EmptyService, perhaps we should use that instead?

TODO

After deciding whether this approach is feasible the following needs to be done:

  • Implement add_optional_service onto the Router in a similar manner
  • Documentation

@hlb8122 hlb8122 changed the title WIP: Add optional service methods Add optional service methods Jun 26, 2020
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

Okay, almost there!

tonic/src/transport/server/mod.rs Outdated Show resolved Hide resolved
tonic/src/transport/server/mod.rs Show resolved Hide resolved
@hlb8122 hlb8122 force-pushed the optional-services branch from 271ed17 to ce7c725 Compare July 10, 2020 16:29
@hlb8122 hlb8122 marked this pull request as ready for review July 10, 2020 16:29
@hlb8122 hlb8122 requested a review from LucioFranco July 10, 2020 16:33
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

:shipit: 🔥

@LucioFranco LucioFranco merged commit 2b997b0 into hyperium:master Jul 10, 2020
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