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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP Server should allow custom concurrency models #13428

Merged
merged 1 commit into from
May 6, 2023

Conversation

stakach
Copy link
Contributor

@stakach stakach commented May 4, 2023

Splitting out this function makes it simple to implement a custom HTTP::Server using a Fiber pool i.e.

require "fiberpool"

class MyServer < HTTP::Server
  @my_pool : FiberPool = FiberPool.new(200)

  protected def dispatch(io)
    @my_pool.run { handle_client(io) }
  end
end

for example

Splitting out this function makes it simple to implement a custom HTTP::Server using a Fiber pool
i.e.

```crystal
require "fiberpool"

class MyServer < HTTP::Server
  @my_pool : FiberPool = FiberPool.new(200)

  protected def dispatch(io)
    @my_pool.run { handle_client(io) }
  end
end
```

for example
Copy link
Contributor

@HertzDevil HertzDevil left a comment

Choose a reason for hiding this comment

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

I am not entirely sure if it is a good idea to expect people to inherit from HTTP::Server. This is not a public API, so it has no stability guarantees and can be easily broken by things like #13080, yet this is not something meant to be publicly callable from the outside either. I am approving this as a refactor, but please note that this can break in a minor release without prior notice given the current state of affairs.

If we decide that protected or private methods can be part of a public API because of inheritance, we should find a way to show them in the docs in the future (e.g. something with the opposite effect of :nodoc:). Shard authors might want that too.

@straight-shoota straight-shoota added this to the 1.9.0 milestone May 4, 2023
@stakach stakach mentioned this pull request May 4, 2023
@straight-shoota straight-shoota merged commit 2886546 into crystal-lang:master May 6, 2023
@stakach stakach deleted the patch-9 branch July 24, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants