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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MTL Lambda #60

Merged
merged 28 commits into from
Dec 13, 2021
Merged

MTL Lambda #60

merged 28 commits into from
Dec 13, 2021

Conversation

armanbilge
Copy link
Member

Here we go again 馃槅

This takes the idea in #51 (comment) about MTL-based middlewares for a spin.

The magic in this abstraction is provided by:

trait LambdaEnv[F[_], Event] {
  def event: F[Event]
  def context: F[Context[F]]
}

As long as you have a LambdaEnv, you can access the event and context from anywhere in the lambda. I think this is fantastic. It improves composability yet one more time: for example, now the http4s and CloudFormation lambdas no longer have to explicitly pass around Context[F] objects so that their routes/handlers have access to them. In general, you can build up your middleware stack however you want without worrying if the inputs of your middlewares line up: every middleware independently has full access to the LambdaEnv.

}
}

def handler(implicit env: LambdaEnv[IO, Event]): Resource[IO, IO[Option[Result]]]
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is the most confusing aspect of this change: The Resource is run once, to create an IO that will be run many times. The IO is the handler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, this is unsafe, because it would let you access the event and context while building the resource stack, which is not right. I knew those nulls were a bad sign 馃槄 . Fixing.

@bpholt
Copy link
Member

bpholt commented Nov 23, 2021

This looks interesting! I'll try to publish it locally and pull it into the two Lambdas I've been playing with (one regular one, and one custom CFN resource) and report back. (Hopefully tomorrow.)

@armanbilge
Copy link
Member Author

Thanks! No rush, and grateful for your patience with all the churn while we figure this out 馃榿 having you try these designs out with your lambdas has been invaluable, much appreciated!

If it helps, I just published this branch as:

"com.armanbilge" %%% "feral-lambda" % "0.1-2f94f32" // and always friends!

@bpholt
Copy link
Member

bpholt commented Dec 2, 2021

Finally got around to trying this, so I'm going to add some comments on the code in no particular order.

Copy link
Member

@bpholt bpholt left a comment

Choose a reason for hiding this comment

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

Working with LambdaEnv[F, Input] => F[Option[Output]] can be a little awkward when you want to define an anonymous function but make the LambdaEnv[F, Input] available implicitly to the body of the function. I started working around it by defining it as a Kleisli, which makes me wonder if it should actually be a Kleisli in the library after all.


object TracedLambda {

def apply[F[_]: MonadCancelThrow, Event: KernelSource, Result](entryPoint: EntryPoint[F])(
Copy link
Member

Choose a reason for hiding this comment

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

I think if we tweak this a bit, we'll get better type inference downstream:

def apply[F[_] : MonadCancelThrow, Event, Result](entryPoint: EntryPoint[F])
                                                 (lambda: Span[F] => F[Option[Result]])
                                                 (implicit 
                                                  env: LambdaEnv[F, Event],
                                                  KS: KernelSource[Event]): F[Option[Result]]

As written now, it seems to get confused about what the Event type is, I think because it binds the KernelSource[Event] (using apiGatewayProxyEventV2KernelSource) before looking for the LambdaEnv[F, Event], which, if Event is not ApiGatewayProxyEventV2, won't be available.

Explicitly putting the implicit LambdaEnv[F, Event] parameter first lets it use that to bind Event, and then it correctly finds the emptyKernelSource low-priority implicit value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha! Very nice!

import io.circe.Decoder
import io.circe.Encoder
import cats.effect.IOLocal

abstract class IOLambda[Event, Result](
implicit private[lambda] val decoder: Decoder[Event],
private[lambda] val encoder: Encoder[Result]
Copy link
Member

Choose a reason for hiding this comment

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

I noticed that the Encoder[INothing] is missing from this PR. Was that intentional? I had to add it manually to get things working with a Result of INothing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah, I didn't merge that branch into this one. We should probably start landing some of these 馃槄

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, agreed. I would love to get an alpha version of this published under "org.typelevel" soon!

}
}

object LambdaEnv {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to add a summoner and implicit instances for Kleisli and friends:

def apply[F[_], A](implicit env: LambdaEnv[F, A]): LambdaEnv[F, A] = env

implicit def kleisliLambdaEnv[F[_] : Functor, A, B](implicit env: LambdaEnv[F, A]): LambdaEnv[Kleisli[F, B, *], A] =
  env.mapK(Kleisli.liftK)

implicit def optionTLambdaEnv[F[_] : Functor, A](implicit env: LambdaEnv[F, A]): LambdaEnv[OptionT[F, *], A] =
  env.mapK(OptionT.liftK)

implicit def eitherTLambdaEnv[F[_] : Functor, A, B](implicit env: LambdaEnv[F, A]): LambdaEnv[EitherT[F, B, *], A] =
  env.mapK(EitherT.liftK)

implicit def writerTLambdaEnv[F[_] : Applicative, A, B : Monoid](implicit env: LambdaEnv[F, A]): LambdaEnv[WriterT[F, B, *], A] =
  env.mapK(WriterT.liftK[F, B])

The Kleisli instance comes in handy when tracing with Kleisli[F, Span[F], *]. Not sure if the others are needed but I've noticed they get added in a lot of TypeLevel projects and I don't see any reason not to include them.

Copy link
Member Author

Choose a reason for hiding this comment

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

馃憤 for sure! Good to have.

client: Client[F],
handler: CloudFormationCustomResource[F, Input, Output])(
implicit
env: LambdaEnv[F, CloudFormationCustomResourceRequest[Input]]): F[Option[Unit]] = {
Copy link
Member

Choose a reason for hiding this comment

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

Should F[Option[Unit]] be F[Option[INothing]]?

@bpholt
Copy link
Member

bpholt commented Dec 2, 2021

FYI, the POC code using this branch is available here for a plain-ol' AWS Lambda and here for a custom CloudFormation resource.

@armanbilge
Copy link
Member Author

armanbilge commented Dec 2, 2021

Working with LambdaEnv[F, Input] => F[Option[Output]] can be a little awkward when you want to define an anonymous function but make the LambdaEnv[F, Input] available implicitly to the body of the function.

Do you have an example of this? I was thinking something like this, but maybe it doesn't work so well.

{ implicit env =>
  // use it implicitly
}

I started working around it by defining it as a Kleisli, which makes me wonder if it should actually be a Kleisli in the library after all.

Yes, that's the heart of the matter :) I assume you mean like we considered in #51 (comment)?

The Kleisli and MTL approach are not mutually exclusive. If you have a Kleisli with (Event, Context[F]) then you can derive a LambdaEnv[F, Event]. So that might be one option. Edit: no, that's not quite right, the kleisli would become the effect type. In any case, this wouldn't work with IOLocal but maybe that's moot.

So overall, how do you like this MTL approach vs the approach in #51?

@bpholt
Copy link
Member

bpholt commented Dec 2, 2021

Do you have an example of this? I was thinking something like this, but maybe it doesn't work so well.

{ implicit env =>
 // use it implicitly
}

That works fine if the return type is LambdaEnv[F, Input] => F[Option[Output]] directly, but I found that it required an explicit type on env when used e.g. in a for-comprehension returning Resource[F, LambdaEnv[F, Input] => F[Option[Output]]] like this one:

  private def resources[F[_] : Async : Console]: Resource[F, LambdaEnv[F, CloudFormationCustomResourceRequest[DatabaseMetadata]] => F[Option[Unit]]] =
    Resource.eval(Slf4jLogger.create[F]).flatMap { implicit logger =>
      for {
        client <- httpClient[F]
        entryPoint <- Resource.eval(Random.scalaUtilRandom[F]).flatMap { implicit r => XRay.entryPoint[F]() }
        secretsManager <- SecretsManagerAlg.resource[F, Kleisli[F, Span[F], *]]
      } yield Kleisli { implicit env: LambdaEnv[F, CloudFormationCustomResourceRequest[DatabaseMetadata]] =>
        TracedLambda[F, CloudFormationCustomResourceRequest[DatabaseMetadata], Unit](entryPoint) { span =>
          val tracedClient = NatchezMiddleware.client(client.translate(Kleisli.liftK[F, Span[F]])(Kleisli.applyK(span)))
          CloudFormationCustomResource(tracedClient, PostgresqlDatabaseInitHandlerImpl(secretsManager)).run(span)
        }
      }.run
    }

@armanbilge
Copy link
Member Author

Thanks for that example!

So after playing with your RabbitMQ lambda a bit in Dwolla/rabbitmq-topology-backup#77 I think we can avoid issues like above by binding to IO just a bit earlier. So for example that snippet you posted above I think should just be the handler, and everything there should be in IO.

I think it is very reasonable/realistic/practical to acquire resources and patch together the lambda handler directly in IO. The important thing is that the handler logic and resources themselves are all written in F, just needs a big of glue at the end :)

What do you think?

armanbilge and others added 3 commits December 3, 2021 15:48
Co-authored-by: Brian P. Holt <bholt@dwolla.com>
Co-authored-by: Brian P. Holt <bholt@dwolla.com>
@armanbilge armanbilge mentioned this pull request Dec 9, 2021
6 tasks
@bpholt
Copy link
Member

bpholt commented Dec 12, 2021

I'm not sure what I was doing differently before, but I don't seem to have that problem doing

} yield { implicit env =>

with handlerF defined as

def handlerF[F[_] : Async]: Resource[F, LambdaEnv[F, RabbitMQConfig] => F[Option[INothing]]]

now, so that's good news.

(I still don't think I would want to move the resource acquisition stuff into IO, but that's just a personal preference鈥擨 don't think it's actually a dealbreaker.)

@bpholt
Copy link
Member

bpholt commented Dec 12, 2021

Unless anyone sees any blockers, I think we should run with this design, cut an alpha release, and let people start playing with it. As early in the project as we are, I think we have flexibility to change things quite a bit if we discover this approach doesn't work for some reason.

@kubukoz
Copy link
Member

kubukoz commented Dec 13, 2021

@armanbilge bring it on :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants