Skip to content

edits for tutorial draft#1

Open
katyhoward wants to merge 1 commit into
masterfrom
new_tut
Open

edits for tutorial draft#1
katyhoward wants to merge 1 commit into
masterfrom
new_tut

Conversation

@katyhoward
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@hjet hjet left a comment

Choose a reason for hiding this comment

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

Some suggestions

Comment thread docker-compose.yml
- MONGO_PASSWORD=$MONGO_PASSWORD
- MONGO_HOSTNAME=$MONGO_HOSTNAME
- MONGO_PORT=$MONGO_PORT
- MONGO_DB=$MONGO_DB
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would set the MONGO_HOSTNAME variable to db explicitly here, instead of setting it to db in the config parameter file. Would either leave it blank/unset or omit it from that file entirely. db is a "Compose-level" variable, so referring to it at the Compose level makes more sense and is cleaner, IMO. See comment on Tutorial markdown.

Comment thread docker-compose.yml
container_name: db
restart: unless-stopped
environment:
- MONGO_INITDB_ROOT_USERNAME=user
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would avoid checking these (even more) sensitive parameters into version control. Would create a separate .env file (or use the same file) and externalize these from the Dockerfile.

Comment thread app/wait-for.sh
@@ -0,0 +1,79 @@
#!/bin/sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would maybe add a comment in the script linking out to the original on GH.

Comment thread .dockerignore
README.md
.gitignore
.env
db-init.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would check db-init.js into version control (since it is code that gets executed), and modify the script so it pulls config/sensitive info from the already configured .env file. All config should be centralized and removed from code (and all code that is run should be checked in to VC).

Comment thread .gitignore
#DynamoDB Local files
.dynamodb/
.dynamodb/
db-init.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See above comment.

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.

2 participants