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

BL-1840 Make web_concurrency configurable #4249

Merged
merged 1 commit into from Feb 26, 2024

Conversation

sensei100
Copy link
Contributor

I will follow up this PR with one to charts to add the environment variable there.

RUBY_YJIT_ENABLE="1"

USER root

ARG SECRET_KEY_BASE
ARG WEB_CONCURRENCY
Copy link
Member

Choose a reason for hiding this comment

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

I don't think just a docker build arg alone will work because build args go away after the build and it needs to stay as an system env. I was thinking maybe a combo of a build arg plus the env would do the trick. though, i'm not 100 sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dkinzer I added the ENV Variable to the charts deployment template. Is that not enough?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately it's not because the point of this is to make it configurable locally too and the chart change would just manage the deployed change. Also ARG doesn't work alone because ARG is supposed to be exclusively for branching the build process. Hmm.. Maybe we can just add --env to run in Makefile and that would be enough for local override?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dkinzer I added it as an env var to the make run command. I did run it locally and the variable was set to what I expected it to be.

@dkinzer dkinzer merged commit b1d88ed into main Feb 26, 2024
3 checks passed
@dkinzer dkinzer deleted the BL-1840-web-concurrency-env-var branch February 26, 2024 16:10
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