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

Use ServiceDescriptor within MethodDescriptor #2127

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 21, 2024

Motivation:

MethodDescriptor uses a string to represent the fully qualified name of the service the method belongs to. ServiceDescriptor also represents a fully qualified name of a service. It'd make more sense for MethodDescriptor to use a ServiceDescriptor instead of a string to describe the service.

Modifications:

  • ServiceDescriptor now stores the fully qualified service name (as this is generally more useful) and computes the package and unqualified service name.
  • ServiceDescriptor can now be created from the fully qualified name or from a separate package and service name.
  • MethodDescriptor now holds a ServiceDescriptor instead of a String for the service

Result:

More coherent API

Motivation:

MethodDescriptor uses a string to represent the fully qualified name of
the service the method belongs to. ServiceDescriptor also represents a
fully qualified name of a service. It'd make more sense for
MethodDescriptor to use a ServiceDescriptor instead of a string to
describe the service.

Modifications:

- ServiceDescriptor now stores the fully qualified service name (as this
  is generally more useful) and computes the package and unqualified
  service name.
- ServiceDescriptor can now be created from the fully qualified name or
  from a separate package and service name.
- MethodDescriptor now holds a ServiceDescriptor instead of a String for
  the service

Result:

More coherent API
@glbrntt glbrntt added the ⚠️ semver/major Breaks existing public API. label Nov 21, 2024
@glbrntt
Copy link
Collaborator Author

glbrntt commented Nov 22, 2024

API breakage is expected and okay.

@glbrntt glbrntt merged commit ef48ef8 into grpc:main Nov 22, 2024
42 of 45 checks passed
@glbrntt glbrntt deleted the v2/use-service-desciptor-in-method branch November 22, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants