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

using defined Puma config #205

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

JackDanger
Copy link

This changes threads the gemstash_env.config options for Puma through to the Puma::CLI.new call.

Prior to this change :puma_workers and :puma_threads were ignored when booting Gemstash via the CLI

@JackDanger JackDanger force-pushed the jackdanger/pass-through-puma-config branch from df148bf to e68b750 Compare December 6, 2018 03:56
JackDanger added a commit to Gusto/gemstash-public-fork-archive that referenced this pull request Dec 6, 2018
This changes threads the `gemstash_env.config` options for Puma through
to the `Puma::CLI.new` call.

Prior to this change `:puma_workers` and `:puma_threads` were ignored
when booting Gemstash via the CLI
@JackDanger JackDanger force-pushed the jackdanger/pass-through-puma-config branch from e68b750 to a031473 Compare December 6, 2018 04:04
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

It seems one of the new methods was not called.

@@ -33,12 +33,24 @@ def puma_config
File.expand_path("../../puma.rb", __FILE__)
end

def store_pidfile
gemstash_env.pidfile = pidfile?
end
Copy link
Member

Choose a reason for hiding this comment

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

This method and #pidfile? are unused. Perhaps it's not meant to be a part of this changeset?

Copy link
Contributor

Choose a reason for hiding this comment

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

Noted and removed.

@phanle
Copy link
Contributor

phanle commented Aug 12, 2019

@olleolleolle Let me know if further updates are required.

Julia Lee and others added 5 commits August 12, 2019 14:32
This effectively applies commit 7624401
When gusto/redis is merged in upstream, then this commit becomes unnecssary.
For now gusto's gemstash fork is in an unideal inbetween state
@bronzdoc
Copy link
Member

Would like to test this before merge 😬

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