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

Refactor load_mongodump.sh script #150

Merged
merged 15 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,6 @@ dmypy.json

# Pyre type checker
.pyre/

# docker-compose-dev.yml
Copy link
Member

Choose a reason for hiding this comment

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

should both docker-compose files be excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a good idea. I have excluded both docker-compose files to be excluded.

docker-compose.yml
shankari marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 19 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# A simple and stupid dashboard for e-mission
# A simple dashboard for e-mission

Issues: Since this repository is part of a larger project, all issues are tracked in the central docs repository. If you have a question, as suggested by the open source guide, please file an issue instead of sending an email. Since issues are public, other contributors can try to answer the question and benefit from the answer.

Expand Down Expand Up @@ -70,6 +70,24 @@ Note that this expects a standard setup with:
- this repository checked out under the `em-public-dashboard` directory, which makes the database name `em-public-dashboard_db_1`
- the incoming mongodump is in tar gz format. This should be true of all canbikeco dumps, you may need to change the `tar xvf` to `unzip` otherwise. The mongo container typically doesn't have zip installed, so using tar is more portable.

## Working with `docker compose` and `.gitignore`

### Using `docker compose`

When working with `docker compose`, it's generally recommended to avoid committing changes to the `docker-compose.dev.yml` file, especially if you're running the `./load_mongodump <dump tar>` script. This file is typically configured to work in a specific way for your development environment, and changes might not be applicable or useful for others working on the same project.

### `.gitignore` Configuration

To streamline your workflow, we have added the `docker-compose.dev.yml` file to the `.gitignore` file. This means that by default, changes to `docker-compose.dev.yml` will not be tracked by Git. This setup helps to avoid unnecessary commits and ensures that your `docker-compose.dev.yml` remains consistent with the intended configuration for the project.

### Committing Changes to `docker-compose.dev.yml`

If you do need to make changes to `docker-compose.dev.yml` and want to commit those changes, you can override the ignore settings by using the following Git command:

```bash
git add -f docker-compose.dev.yml
```

**If you have a non-standard setup, please use your expertise to change the script appropriately.**

#### Happy visualizations!
Expand Down Expand Up @@ -121,7 +139,6 @@ You may need to increase the resources avaliable to Docker if:
- you believe you've loaded the data but there is none when running the notebooks
- the notebook can't connect to the database
- when you try and start the container for the database it exits with code 14
-

## Large Dataset Workaround

Expand Down
2 changes: 1 addition & 1 deletion docker-compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ services:
depends_on:
- db
environment:
- DB_HOST=db
Copy link
Member

Choose a reason for hiding this comment

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

would it be helpful to make these same changes in docker-compose.yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent of using load_mongodump.sh is to load the dataset into mongo db.
A couple of observations:

  • We are loading the config file in load_mongodump.sh from docker-compose.dev.yml.
    CONFIG_FILE="$SCRIPT_DIR/../../docker-compose.dev.yml"
  • We would need to make some changes in load_mongodump.sh if we want to configure for both docker-compose.yml and docker-compose.dev.yml.
  • We can anyways load the dataset with the current changes for the public dashboard db, do you think the additional changes would be a necessity? Moreover, it's implemented as in with op-admin dashboard.

Please let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

My thought was that the updated default with the formatting might make it easier to switch that dataset in use when editing the docker file (ie update from mongodb://db/DB_NAME to mongodb://db/openpath_prod_open_acess instead of from db, but it sounds like there are more technical reasons that differ between the two files, so I'm ok with leaving the other dockerfile alone

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, users don't need to rename DB_HOST=mongodb://db/DB_NAME manually; the script edits the file directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverting back to DB_HOST=db such that it's consistent between docker-compose.dev.yml and docker-compose.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, I don't understand why you reverted this change. I am also not sure how it works, given that load_mongodump.sh expects a mongodb:// URL so that it can change the DB_NAME appropriately, I bet that this was not tested, because I don't see how it could work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have documented my testing below #150 (comment)
In the load_mongodump.sh:

# Update the docker-compose configuration file with the actual DB_HOST
DB_HOST="mongodb://db/$DB_NAME"
sed -i.bak "s|DB_HOST:.*|DB_HOST: $DB_HOST|" "$CONFIG_FILE"

I understand, it would assign mongdb://db$DB_NAME to the configuration file.

I have also re-done testing and updated detailed logs below. #150 (comment)

- DB_HOST=mongo://db/DB_NAME
Copy link
Member

Choose a reason for hiding this comment

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

should we make this change in the other docker-compose file as well?

Copy link
Member

Choose a reason for hiding this comment

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

This also needs to be mongodb and not mongo

- WEB_SERVER_HOST=0.0.0.0
- CRON_MODE=
- STUDY_CONFIG=stage-program
Expand Down
72 changes: 67 additions & 5 deletions viz_scripts/docker/load_mongodump.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,71 @@
#!/bin/bash

# Directory of the script
SCRIPT_DIR="$(dirname "$0")"

# Path to the configuration file (one level up)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should be two levels up

CONFIG_FILE="$SCRIPT_DIR/../../docker-compose.dev.yml"

# Check if the correct number of arguments is provided
if [ "$#" -ne 1 ]; then
echo "Usage: $0 <mongodump-file>"
echo " <mongodump-file> : The path to the MongoDB dump file to be restored."
echo " run git add -f <docker compose file> after using this command"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this since it contradicts the README instructions. I know it was in the original PR, but was removed in response to review feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

@iantei the entire if check does not contradict the README, only the last echo. I will fix this before merging so that we don't have to wait for yet another round of reviews.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have re-introduced the check while removing the last echo statement.

exit 1
fi

MONGODUMP_FILE=$1

echo "Copying file to docker container"
docker cp $MONGODUMP_FILE em-public-dashboard-db-1:/tmp
# Print debug information
echo "Script Directory: $SCRIPT_DIR"
echo "Configuration File Path: $CONFIG_FILE"
echo "MongoDump File Path: $MONGODUMP_FILE"

# Check if the provided file exists
if [ ! -f "$MONGODUMP_FILE" ]; then
echo "Error: File '$MONGODUMP_FILE' does not exist."
exit 1
fi

# Check if the configuration file exists
if [ ! -f "$CONFIG_FILE" ]; then
echo "Error: Configuration file '$CONFIG_FILE' does not exist."
exit 1
fi

# Print details about the configuration file
echo "Configuration file details:"
ls -l "$CONFIG_FILE"

# Extract the database name from the mongodump file
DB_NAME=$(tar -tf "$MONGODUMP_FILE" | grep '^dump/' | sed 's|^dump/||' | awk -F'/' '{if (NF > 0) {print $1; exit}}')

# Output the database name
echo "$DB_NAME"

if [ -z "$DB_NAME" ]; then
echo "Error: Failed to extract database name from mongodump."
exit 1
fi

echo "Database Name: $DB_NAME"

# Update the docker-compose configuration file with the actual DB_HOST
DB_HOST="mongodb://db/$DB_NAME"
sed -i.bak "s|DB_HOST:.*|DB_HOST: $DB_HOST|" "$CONFIG_FILE"

echo "Updated docker-compose file:"
cat "$CONFIG_FILE"

echo "Copying file to Docker container"
docker cp "$MONGODUMP_FILE" em-public-dashboard-db-1:/tmp

FILE_NAME=$(basename "$MONGODUMP_FILE")

echo "Clearing existing database"
docker exec em-public-dashboard-db-1 bash -c "mongo $DB_NAME --eval 'db.dropDatabase()'"

FILE_NAME=`basename $MONGODUMP_FILE`
echo "Restoring the dump from $FILE_NAME to database $DB_NAME"
docker exec -e MONGODUMP_FILE=$FILE_NAME em-public-dashboard-db-1 bash -c "cd /tmp && tar xvf $FILE_NAME && mongorestore -d $DB_NAME dump/$DB_NAME"

echo "Restoring the dump from $FILE_NAME"
docker exec -e MONGODUMP_FILE=$FILE_NAME em-public-dashboard-db-1 bash -c 'cd /tmp && tar xvf $MONGODUMP_FILE && mongorestore'
echo "Database restore complete."