-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add Helm chart and deployment method #848
Conversation
…OIT into first-draft-of-new-helm-chart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a few issues and questions related to the .env and documentation.
.env
Outdated
@@ -152,9 +151,7 @@ DATE_FORMAT="Y-m-d" | |||
|
|||
###> base url ### | |||
# Base URL for client callbacks | |||
BASE_URL="" | |||
# Webpack public path | |||
WEBPACK_PUBLIC_PATH="/build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Helm require that you set these valuse in .env rather than .env.local? Removing this variable will default the build path to /build
(as seen in https://github.com/ucfopen/UDOIT/blob/dev/v3-3-1/webpack.config.js#L13). If BASE_URL
is set to http://127.0.0.1:8000/udoit3
, then WEBPACK_PUBLIC_PATH
needs to be set to /udoit3/build
Or do these changes even matter, since you're configuring everything through the values.yml
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit I do not recall why this was removed/changed at all. Looking. at the commits, it seems the WEBPACK_PUBLIC_PATH
variable was not in the original branch we worked off of? 9519c9e
I may be misunderstanding.
In any case for our purposes, they do not actually matter. I will fix this to match what is there now
I resolved the conflicts in a new branch, but I was unable to merge those changes into your fork. I'm closing this PR and making a new one. |
This pull request will close issue #847