-
Notifications
You must be signed in to change notification settings - Fork 1
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 ConsulDiscoverable typeclass and Scala 2 macro implementation #217
Conversation
import scala.annotation.implicitNotFound | ||
|
||
@implicitNotFound("Instances are only available for Smithy4s Services annotated with @discoverable") | ||
trait ConsulDiscoverable[Alg[_[_]]] { |
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.
I'm open to bikeshedding on the names here. This typeclass will have macro-instances for Smithy4s services annotated with the @discoverable
trait. The host
, uriAuthority
, and uri
values will be derived from the serviceName
value on the @discoverable
trait, e.g. uri
will return consul://${serviceName}
.
This will typically be used like this:
def smithyClient[F[_], Alg[_[_, _, _, _, _]]](client: Client[F], // http4s client
consulMiddleware: ConsulMiddleware[F], // middleware from this library
service: Service[Alg], // generated by smithy4s
)
(implicit C: ConsulDiscoverable[service.Impl]): Resource[F, service.Impl[F]] =
SimpleRestJsonBuilder.apply(service)
.client(NatchezMiddleware.clientWithAttributes(client)(
"rpc.system" -> "smithy",
"peer.service" -> ConsulDiscoverable[service.Impl].host,
))
.uri(ConsulDiscoverable[service.Impl].uri)
.middleware(consulMiddleware)
.resource
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.
And just to be clear, this does not work, because the macro can't resolve what service
is at compile time:
def smithyClient[F[_], Alg[_[_, _, _, _, _]]](client: Client[F], // http4s client
consulMiddleware: ConsulMiddleware[F], // middleware from this library
service: Service[Alg], // generated by smithy4s
): Resource[F, service.Impl[F]] =
SimpleRestJsonBuilder.apply(service)
.client(NatchezMiddleware.clientWithAttributes(client)(
"rpc.system" -> "smithy",
"peer.service" → HostFromService(service),
))
.uri(UriFromService(service))
.middleware(consulMiddleware)
.resource
Implementing it as a typeclass does work, because there is a stable location (outside the function above) where the service is known and the ConsulDiscoverable
instance can be generated and then passed implicitly into the function.
…othing relies on them being in a specific place
…to be package-private
42191b8
to
214e0ec
Compare
def hostToConsulDiscoverableExpr(tpe: Tree): Host => c.Expr[ConsulDiscoverable[Alg]] = host => | ||
c.Expr[ConsulDiscoverable[Alg]]( | ||
q""" | ||
new _root_.com.dwolla.consul.smithy4s.ConsulDiscoverable[$tpe] { |
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.
My mental model of how macros work is that it's like the code block is copy/pasted into the spot where the macro is invoked. The imports, etc., that are in effect in that spot are what controls how types are resolved. So e.g. a prior version used the .some
enhancement method from Cats (uriAuthority.some
) instead of wrapping the value in an Option
as below, which worked in places where the Cats syntax had been imported, but failed otherwise.
To avoid that kind of issue or other issues with ambiguous imports, I fully qualified everything in the quasiquote so that it should work regardless of context.
Adding the
ConsulDiscoverable[Alg]
typeclass allows the macro-derivedUri
,Uri.Authority
, andHost
values to be used with generic service types, e.g. in functions where the service is passed in and not a literal value.I ran into this when trying to extract out some common code initializing Smithy4s clients from one of our internal codebases, so this should open up that possibility.