-
Notifications
You must be signed in to change notification settings - Fork 75
replace yaml-merge with yq, fix quickstart deployment docs. Closes #1780 #1782
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
Conversation
brollb
left a comment
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 added a couple comments below
docs/deployment/quick_start.rst
Outdated
| openssl rsa -in deepforge_keys/private_key -pubout > deepforge_keys/public_key | ||
| export TOKEN_KEYS_DIR="$(pwd)/deepforge_keys" | ||
| Then create file called ``production-docker-compose.yml`` (for using the keys generated above) with the following |
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.
Putting this file inline is not ideal as it will likely get out-of-date pretty easily. It seems more promising to use something like docker-compose run and specify the env variables there.
| server: | ||
| entrypoint: /.deployment/dev-entrypoint.sh | ||
| environment: | ||
| - "NODE_ENV=production" |
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 am guess this is appended so it overrides the earlier NODE_ENV from the other docker-compose file. Is that right?
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.
Yeah. But it might not be necessary, the only reason I added it was to make quick start easier (without user accounts) with adding NODE_ENV=default in docker/docker-compose.yml. It might not be needed if we use docker-compose run.
docker-compose --file docker-compose.yml run -p 8888:8888 -p 8889:8889 -e "NODE_ENV=default" server1. Revert to docker-compose.yml with ${TOKEN_KEYS_DIR} mount
2. Remove NODE_ENV=default from docker-compose.yml and fix key removal
3. Change documentation quickstart with docker using docker-compose run
docs/deployment/quick_start.rst
Outdated
| .. code-block:: bash | ||
| docker-compose up | ||
| docker-compose --file docker-compose.yml run -v "${TOKEN_KEYS_DIR}:/token_keys" -p 8888:8888 -p 8889:8889 -e "NODE_ENV=production" server |
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 don't think we need -e "NODE_ENV=production" here, do we?
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 don't think we need it.
No description provided.