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

Adding falcon integration #1677

Closed
wants to merge 3 commits into from

Conversation

horaciob
Copy link
Contributor

@horaciob horaciob commented Jan 8, 2021

#1639
Dont know if I have to do something else, I saw puma's integration, it works on local.

Puma will be default if exist.

I've tried with jruby but looks like falcon has some issues with jruby

@horaciob horaciob mentioned this pull request Jan 8, 2021
@namusyaka namusyaka requested a review from jkowens February 12, 2021 19:59
@namusyaka
Copy link
Member

@jkowens Could you take a look?

@jkowens
Copy link
Member

jkowens commented Feb 12, 2021

Looks good to me, except that Falcon server doesn't shutdown gracefully like our other Rack servers when hitting ctrl + c:

/lib/sinatra/base.rb:1478:in `quit!': undefined method `stop' for #<Falcon::Server:0x00007f9729945620> (NoMethodError)

@horaciob is there a way to make that work?

@horaciob
Copy link
Contributor Author

They have some issues related to gracefull (shutdown and restart), let me see if are related with this or i can do something to avoid this.

@ioquatix
Copy link
Contributor

Who is calling stop on SIGINT?

@namusyaka
Copy link
Member

Any progress?

@ioquatix
Copy link
Contributor

This is the problem code:

https://github.com/sinatra/sinatra/blob/master/lib/sinatra/base.rb#L1481-L1489

It assumes the server responds to #stop. So, for this to work, either we need to realise not all servers respond to stop! or stop and write a wrapper which invokes the correct method, or we need to add #stop to Falcon::Server some how.

@jkowens
Copy link
Member

jkowens commented Jul 18, 2022

Replaced by #1794

@jkowens jkowens closed this Jul 18, 2022
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.

4 participants