-
Notifications
You must be signed in to change notification settings - Fork 0
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
Log Records Seeding Script #74
Conversation
db-init/seed.sql
Outdated
INSERT INTO log_records ( | ||
employee_id, | ||
resident_first_name, | ||
resident_last_name, |
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.
this needs to be updated to resident_id but we need to wait for #50 to be merged first
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.
it can never be escaped
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.
we can ignore this and merge what you have for now
db-init/seed.sql
Outdated
flagged, | ||
attn_to, | ||
note, | ||
tags, |
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.
same here as well, need to wait for your tags PR to be merged
db-init/seed.sql
Outdated
tags, | ||
building ) | ||
|
||
SELECT |
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.
would we able to do something similar to set up residents and a few dummy accounts in the db? It would definitely help set up a testing env fairly quickly and we can include an admin account here and in Firebase (so we don't have to keep creating accounts on fresh runs of the containers)
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.
Yes
db-init/seed.sql
Outdated
1, | ||
(ARRAY['Amazing note','This is a note','Bad note','Decent Note','Cool note'])[floor(random() * 5 + 1)], | ||
ARRAY['tag1', 'tag2'], | ||
(ARRAY['Building A','Building B','Building C'])[floor(random() * 3 + 1)] |
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.
buildings are:
144, 362, 402
I think it would be beneficial to at least share these files in Slack so they can be used as a template for other tables Just so we're not waiting on a merge |
db-init/seed.sql
Outdated
INSERT INTO log_records ( | ||
employee_id, | ||
resident_first_name, | ||
resident_last_name, |
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.
we can ignore this and merge what you have for now
db-init/seed.sql
Outdated
ARRAY['tag1', 'tag2'], | ||
(ARRAY['Building A','Building B','Building C'])[floor(random() * 3 + 1)] | ||
|
||
FROM generate_series(1, 50); -- Change this to get a different amount of rows |
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.
can we move this to a env file in db-init? I'd like to avoid bi-accidently pushing changes to this file
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.
The number of rows?
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
seeding/seed.sh
Outdated
|
||
#Get absolute path to seeding dir | ||
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
cat $SCRIPT_DIR/seed.sql | docker exec -i SHOW-database /bin/bash -c 'psql -U postgres -d show_db' |
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.
below was an error I ran into running the script on Windows:
OCI runtime exec failed: exec failed: unable to start container process: exec: "C:/Program Files/Git/usr/bin/bash": stat C:/Program Files/Git/usr/bin/bash: no such file or directory: unknown
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.
The fix is to double the slashes so that its //bin//bash
, its something to do with Windows interpreting it as a local path instead of a dir on the Docker container (Windows diff)
Idk if this would effect Linux environments, but if it does we can just have to versions of this file, one for Windows and and one for Linux
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.
Tested this locally on Mac (UNIX); had no issues running the script with bash
OR zsh
, so I think we could proceed with the two files soln. I'm thinking we just add a subdir /windows
and /unix
to the seeding
directory OR prepend the OS to the file names themselves
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
cat $SCRIPT_DIR/seed.sql | docker exec -i SHOW-database /bin/bash -c 'psql -U postgres -d show_db' | ||
|
||
echo "Database seeding complete" |
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.
can you display a error message if the above process fails?
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.
you'll know if it fails from the sql logs
also the toast msg won't ever appear
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.
another W Connor moment 😎 small nit for the .env but I'm good to merge this in
Next session you should go over this PR, how to use it, and how to add to it (for example, creating fake residents), it would be extremely helpful for the team :D
seeding/seed.sh
Outdated
#Otherwise, run bash seed.sh | ||
|
||
#Import root env file variables | ||
source ../.env |
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.
would it be possible to have an .env file in the seeding dir instead?
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.
LGTM 🔥
Implementation description
Created a bash script that can be ran to seed the log records table with an arbitrary amount of semi-random records
Add an env file in the seeding directory with these values:
Steps to test
bash seed.sh
(if you're on Windows. include a -w flag)What should reviewers focus on?
Whipped this up with little thought so there's probably room for improvement, but it at least serves as a solid starting point that can be built upon
Checklist