-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: switch to vertx server backend #223
base: main
Are you sure you want to change the base?
Conversation
There issue with non-deterministic EOF is observable by others but so far there is no good explanation (or test method) given (see [1]) therefore for the time being we are switching to vertx http server. TODO: * `OPTIONS` request is not accepted and results in `405 Method Not Allowed` therefore UI is not able to talk to the server but calling `curl` works * at the moment one cannot customise the logs hence they are not displayed when the server is started Verifies #49 [1] http4s/blaze#668
Dispatcher[IO] | ||
.use { dispatcher => | ||
{ | ||
Dependencies | ||
.wire(config) | ||
.use { case Dependencies(httpApi) => | ||
/* | ||
allocates the http api resource, and never releases it (so that the http server is available | ||
as long as our application runs) | ||
*/ | ||
httpApi.resource(dispatcher).useForever | ||
} | ||
} | ||
} | ||
.unsafeRunSync() |
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.
Current approach is based on Vert.x example
Alternatively for
comprehension can be used but it is hard to tell if it is more readable ;)
(for {
dispatcher <- Dispatcher[IO]
servers <- Dependencies
.wire(config)
.map { case Dependencies(httpApi) =>
/*
allocates the http api resource, and never releases it (so that the http server is available
as long as our application runs)
*/
httpApi.resource(dispatcher).useForever.unsafeRunSync()
}
} yield (dispatcher, servers)).useForever.unsafeRunSync()
@adamw WDYT?
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 think we can make it more readable :)
for {
dispatcher <- Dispatcher[IO]
httpApi <- Dependencies.wire(config)
servers <- httpApi.resource(dispatcher)
_ <- Resource.never
} yield whatever
.prependInterceptor(CorrelationIdInterceptor) | ||
// all errors are formatted as json, and there are no other additional http4s routes | ||
.defaultHandlers(msg => ValuedEndpointOutput(http.jsonErrorOutOutput, Error_OUT(msg)), notFoundWhenRejected = true) | ||
// TODO customise the serverLog when available |
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.
fixed in latest tapir
def resource(dispatcher: Dispatcher[IO], endpoints: List[ServerEndpoint[Any with Fs2Streams[IO], IO]], port: Int) = { | ||
Resource.make( | ||
IO.delay { | ||
val vertx = Vertx.vertx() |
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 think we should crate a single shared Vertx
We should check the |
There issue with non-deterministic EOF is observable by others but so far there is no good explanation (or test method) given (see #668) therefore for the time being we are switching to vertx http server.
TODO:
OPTIONS
request405 Method Not Allowed
therefore UI is not able to talk to the server but callingcurl
Verifies #49