fix: escape env variables #62
Conversation
regisb
changed the title
|
I opened this draft PR as a resolution of the issue discussed here: #61 (comment) We still need to decide whether this is the best possible solution. |
regisb
force-pushed
the
regisb/fix-env-escaping
branch
2 times, most recently
from
03ba1be to
8b17339
Compare
With this change, we avoid the problem of having to escape env variables altogether. Previously, env variables were loaded both by bash (in development) and docker-compose (in production). This meant that we had to deal with escaping in both contexts, which was the source of trouble. We resolve this by loading env vars only with bash. Close #61.
regisb
force-pushed
the
regisb/fix-env-escaping
branch
from
8b17339 to
f3fc90d
Compare
|
The rebased and updated PR works with |
| CMD ["/bin/bash", "-c", "set -a && \ | ||
| source /openedx/env/production && \ | ||
| source /openedx/env/development && \ | ||
| npm run start --- --config ./webpack.dev-tutor.config.js"] |
There was a problem hiding this comment.
Do we still need the three dashes here? I can't for the life of me find official documentation on why that's necessary in CMD in the first place, but I thought it was just because it was a separate item in the array. (I could totally be wrong, though.)
There was a problem hiding this comment.
🤔 this change was introduced a couple days ago by @ARMBouhali: https://github.com/overhangio/tutor-mfe/pull/74/files#r1015250142 In my understanding, it's necessary because it's the only way to use the webpack dev config.
Is there an alternative solution?
There was a problem hiding this comment.
At the very least, @ARMBouhali, can you point us to how/where you figured this one out?
|
@arbrandes already mentioned, it's just a little deeper
|
With this change, we avoid the problem of having to escape env variables altogether. Previously, env variables were loaded both by bash (in development) and docker-compose (in production). This meant that we had to deal with escaping in both contexts, which was the source of trouble. We resolve this by loading env vars only with bash.
Close #61.