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

feat: Support pidfile in CLI & Server (defaults to puma.pid) #178

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Spaceghost
Copy link

Expose puma's pidfile feature to CLI & Server.

Notable: defaulting to puma's default pidfile name (puma.pid) might be awkward in the future if this repo migrates to a different application server. I defer to the google for the desired behavior at this level, but default to puma in this pull request.

I needed this for managing the build infrastructure of data ingestion pipelines.

~Spaceghost

Signed-off-by: Johnneylee Jack Rollins <git@spacegho.st>
@garethgeorge
Copy link
Contributor

Thanks @Spaceghost for the contribution! Implementation looks good but I'm interested to better understand your use case and whether there is a general need here.

Can you elaborate a bit more on what you are looking to be able to do with the exposed pid file or possibly open an issue to discuss the feature request in detail?

@Spaceghost
Copy link
Author

It's to add compatibility and support for systemd. I call your executable in non-production environments like ci and the such; I need accurate handles to the puma process to send signals to for restart and stop in the systemd units.

I hope this helps explain, and apologies for not including more context. More is available if that helps.

I think for general utility, it serves many audiences greater with it than without. The burden incurred is obviated by users of the library relying on #pidfile? or the truthiness of the accessor as well as at the operator/sysop levels. Happy either way though! Thank you for the expedient review, @garethgeorge

@garethgeorge garethgeorge requested a review from dazuma July 28, 2023 18:17
@garethgeorge
Copy link
Contributor

garethgeorge commented Jul 28, 2023

Thanks for sharing a bit more about how you're using the FR. I'm curious if you have a need to specifically manage the puma process independently of functions-framework or if this is closer to a work around to ensure that you can signal the two processes?

I'm wondering if you would be able to fork functions-framework in a new process group such that you can signal it and any processes forked by it (i.e. puma) together by signaling the group?

We are evaluating some proposals internally that might move us away from puma for some function signature types (or possibly as a whole) in future releases of functions framework which makes me hesitant to recommend relying on the internals. I'm wondering if there's an approach that will meet your needs that does a better job of treating functions-framework as a blackbox?

@garethgeorge garethgeorge removed the request for review from dazuma July 31, 2023 22:36
@garethgeorge garethgeorge removed their assignment Aug 14, 2023
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.

None yet

2 participants