-
Notifications
You must be signed in to change notification settings - Fork 57
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 service binding for mkCtx returning Resource #567
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,13 @@ trait GeneratedCompanion[Service[*[_], _]] { | |
serverOptions: ServerOptions | ||
): ServerServiceDefinition | ||
|
||
protected def serviceBindingResource[F[_]: Async, A]( | ||
dispatcher: Dispatcher[F], | ||
serviceImpl: Service[F, A], | ||
mkCtx: Metadata => Resource[F, A], | ||
serverOptions: ServerOptions | ||
): ServerServiceDefinition | ||
|
||
final def service[F[_]: Async, A]( | ||
dispatcher: Dispatcher[F], | ||
serviceImpl: Service[F, A], | ||
|
@@ -139,6 +146,13 @@ trait GeneratedCompanion[Service[*[_], _]] { | |
serviceBinding[F, A](dispatcher, serviceImpl, mkCtx, serverOptions) | ||
} | ||
|
||
final def service[F[_]: Async, A]( | ||
serviceImpl: Service[F, A], | ||
f: Metadata => Resource[F, A], | ||
serverOptions: ServerOptions | ||
): Resource[F, ServerServiceDefinition] = | ||
Dispatcher[F].map(serviceBindingResource[F, A](_, serviceImpl, f, serverOptions)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't able to add the error handling that the above method has due to the different signature. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are on 2.5.0-RC1 we can make a breaking change with respect to this method: protected def serviceBinding[F[_]: Async, A](
dispatcher: Dispatcher[F],
serviceImpl: Service[F, A],
mkCtx: Metadata => F[A],
serverOptions: ServerOptions
): ServerServiceDefinition as all the other ones can be implemented in terms of: protected def serviceBinding[F[_]: Async, A](
dispatcher: Dispatcher[F],
serviceImpl: Service[F, A],
mkCtx: Metadata => Resource[F, A],
serverOptions: ServerOptions
): ServerServiceDefinition Having the code gen generate only on method to implement that takes that resource thing. The adapter is:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Furthermore, |
||
|
||
final def service[F[_]: Async, A]( | ||
dispatcher: Dispatcher[F], | ||
serviceImpl: Service[F, A], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really the crux of the PR. I'd like to cut down on the code duplication I just added to this file, but wanted to get general feedback on the PR before spending any time on that.