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

use templates and config maps to get the settings for sidekiq the way we want #976

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

Conversation

orangewolf
Copy link
Contributor

This is simply an alternative approach to the existing way of getting the aux worker configured properly. Trying to find the best pattern.

Copy link
Contributor

@bkiahstroud bkiahstroud left a comment

Choose a reason for hiding this comment

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

I'll need to make a follow up PR replacing the remaining "auxiliary" with "aux", but I don't think that's blocking for this PR

extraEnvVars: *envVars
extraEnvFrom:
- configMapRef:
name: auxilary-worker-env
Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't put this back into pals yet, but in adventist we established this pattern:

  1. Use "aux" instead of "auxiliary"
  2. Put "worker" before "aux"
Suggested change
name: auxilary-worker-env
name: worker-aux-env

apiVersion: v1
kind: ConfigMap
metadata:
name: auxiliary-worker-env
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: auxiliary-worker-env
name: worker-aux-env

extraEnvVars: *envVars
extraEnvFrom:
- configMapRef:
name: auxilary-worker-env
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: auxilary-worker-env
name: worker-aux-env

apiVersion: v1
kind: ConfigMap
metadata:
name: auxiliary-worker-env
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: auxiliary-worker-env
name: worker-aux-env

@@ -4,13 +4,9 @@ if ENV['GOOGLE_OAUTH_PRIVATE_KEY_VALUE'] && !ENV['GOOGLE_OAUTH_PRIVATE_KEY_VALUE
end

if ENV['DATABASE_URL'] && !ENV['DATABASE_URL'].empty?
ENV['DATABASE_URL'] = ENV['DATABASE_URL'].gsub('pool=5', 'pool=30')
ENV['DATABASE_URL'] = ENV['DATABASE_URL'].gsub('pool=5', "pools=#{ENV.fetch('WORKER_CONCURENCY'), 30}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Suggested change
ENV['DATABASE_URL'] = ENV['DATABASE_URL'].gsub('pool=5', "pools=#{ENV.fetch('WORKER_CONCURENCY'), 30}")
ENV['DATABASE_URL'] = ENV['DATABASE_URL'].gsub('pool=5', "pools=#{ENV.fetch('WORKER_CONCURRENCY'), 30}")

else
exec "echo $DATABASE_URL && bundle exec sidekiq"
end
exec "echo $DATABASE_URL && RAILS_MAX_THREADS=$WORKER_CONCURENCY bundle exec sidekiq $WORKER_ARGS"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
exec "echo $DATABASE_URL && RAILS_MAX_THREADS=$WORKER_CONCURENCY bundle exec sidekiq $WORKER_ARGS"
exec "echo $DATABASE_URL && RAILS_MAX_THREADS=$WORKER_CONCURRENCY bundle exec sidekiq $WORKER_ARGS"

labels:
{{- include "hyrax.labels" . | nindent 4 }}
data:
WORKER_CONCURENCY: "5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
WORKER_CONCURENCY: "5"
WORKER_CONCURRENCY: "5"

labels:
{{- include "hyrax.labels" . | nindent 4 }}
data:
WORKER_CONCURENCY: "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
WORKER_CONCURENCY: "1"
WORKER_CONCURRENCY: "1"

labels:
{{- include "hyrax.labels" . | nindent 4 }}
data:
WORKER_CONCURENCY: "5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
WORKER_CONCURENCY: "5"
WORKER_CONCURRENCY: "5"

labels:
{{- include "hyrax.labels" . | nindent 4 }}
data:
WORKER_CONCURENCY: "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
WORKER_CONCURENCY: "1"
WORKER_CONCURRENCY: "1"

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.

2 participants