Unable to change the timeout when using mountService in ServletContextSyntax.scala
Closed this issue · 9 comments
Comment from @rossabaker
Oh, I see the problem
https://github.com/http4s/http4s/blob/master/servlet/src/main/scala/org/http4s/servlet/syntax/ServletContextSyntax.scala
We're setting the timeout there, but not passing it through as a param. That's bad design.
@marcdanie Did you end up with a workaround that you'd like to submit as a PR? Or should I try to come up with something? I'd like to do better here.
I think this just needs a timeout parameter with a default value. The mountService method is an old name as well -- mountRoutes would probably be more sensible.
Hello @rossabaker, is this still open? I can work on it if it is. My understanding is that we just need to add a param with a default value and update the name of the method right?
@yuferpegom Thanks for volunteering! Yes, that sounds like the correct approach for mountService, which we can deprecate.
Doing it for mountHttpApp is harder: we can't add a default parameter without breaking binary compatibility, and mountService calls mountHttpApp. We can do this on the main branch today like you describe, but we'll need to think of a new name if we want to backport to series/0.22.
Hello @rossabaker . So just to double-check we are on the same page. We can just deprecate mountService and then create a new mountRoutes method that will receive a timeout that uses a default value.
As for the mountHttpApp I'm not following you, you mean we can add the default param for this version and merge it to main but eventually, we'd need to also create a new method and deprecate this one? If so, we can just do it now right?
Thanks!
Wow, servlet went from getting no attention to a few PRs today! I'll comment on your PR. Probably easier where there's code to talk about.
I have created a new PR against series/0.23, please take a look when you have a chance and thanks!
@rossabaker should this be closed as completed in http4s/http4s#6037 ?
Yep!