Skip to content

Replace AsyncHttpClient with Ember#121

Open
jsantos17 wants to merge 1 commit into
masterfrom
replace-async-http-client
Open

Replace AsyncHttpClient with Ember#121
jsantos17 wants to merge 1 commit into
masterfrom
replace-async-http-client

Conversation

@jsantos17

Copy link
Copy Markdown
Contributor

No description provided.

: F[A] =
Async.fromFuture(F.delay(mono.asScala.toFuture))
def monoToAsync[F[_]: ConcurrentEffect, A](mono: Mono[A]): F[A] =
fromPublisher[F, A](mono).compile.lastOrError

@jsantos17 jsantos17 Mar 29, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is probably the most significant change. We used to use this to convert, but it looks obsolete given that Scala has supported Java lambdas since 2016. It was also bringing in ancient versions of reactor-netty (the source of this stack trace)

This appeared to be the most straightforward way to convert Monos to IO, given Monos are also publishers.

@jsantos17 jsantos17 requested review from rintcius and wemrysi March 29, 2022 22:06
Comment thread build.sbt
name = Some("Decrypt bad gcp service account json key"))

val AwsSdkVersion = "2.15.34"
val AwsSdkVersion = "2.16.21"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs to be bumped because of netty version change, right? Needs to be done in other libs using the aws sdk as well.

And I think something similar needs to happen for azure as well because they also use netty.

It can be quite a pain to get all of the netty versions line up across all libs. Depending on how far we are with this, it may be easier to roll back https://github.com/precog/quasar-datasource-url/pull/763 et al and publish our own fix directly on top of http4s 0.21.25 so that nothing related to netty changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, we came to the same conclusion and are giving that a go.

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