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

fix(docker): fix passing env vars to running shell #12051

Merged
merged 8 commits into from
Aug 18, 2022

Conversation

laurentlp
Copy link
Contributor

This PR fixes an issue where environment variables were not passed to the shell when starting the Docker container. To fix the issue, we now propagate the env vars so that a user starting the container with -e arguments will see his variables passed to the Botpress binary.

Copy link
Member

@EFF EFF left a comment

Choose a reason for hiding this comment

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

seems good to me, I assume you tested and works.

@slvnperron
Copy link
Member

@sebburon @bassamtantawi-botpress @davidvitora given the history of this one 😆 can you guys please have a look at this

Copy link
Contributor

@davidvitora davidvitora left a comment

Choose a reason for hiding this comment

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

Container failed when running with

sudo docker run -d \
--name=container_name \
-p 3000:3000 \
ghcr.io/botpress/botpress/server:llp_fix_docker_image_env_vars \
bash -c "./duckling & ./bp --auto-migrate"

image

I got an error when running creating the container; I am using my old command to execute Botpress

@davidvitora davidvitora self-requested a review August 17, 2022 15:55
@laurentlp laurentlp requested a review from EFF August 17, 2022 16:17
davidvitora
davidvitora previously approved these changes Aug 17, 2022
Copy link
Contributor

@davidvitora davidvitora left a comment

Choose a reason for hiding this comment

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

After the latest changes, it's working correctly

@sebburon
Copy link
Contributor

Currently testing this on an AWS Hosted cluster.
I will update this when I'm done later today

@bassamtantawi-botpress
Copy link
Member

it is working for me too

Copy link
Contributor

@sebburon sebburon left a comment

Choose a reason for hiding this comment

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

Tested on AWS and it works

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

6 participants