-
Notifications
You must be signed in to change notification settings - Fork 481
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
2018-2019 school import #38432
2018-2019 school import #38432
Conversation
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.
Nice work! The comments were very helpful. This looks good to me overall!
else | ||
School.seed_from_s3 | ||
#else | ||
#School.seed_from_s3 |
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.
just confirming - this is the change to prevent the seeding from happening with the new file before we do a dry run?
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 pretty much -- with school districts I actually did a non-dry run on staging (which appeared to have the entire school district dataset, which test didn't surprisingly), then a non-dry run on production. Basically just wanted to be able to run it manually to keep an eye on it if anything went wrong.
id: row['NCESSCH'].to_i.to_s, | ||
name: row['SCH_NAME'].upcase, | ||
# Four schools with addresses longer than 50 characters (DB column limit) | ||
# Also four schools with second address line longer than 30 characters. |
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.
What happens with those schools?
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'm truncating them (was getting DB errors on import without the truncation, figured cutting these four schools down in length wasn't a huge deal and a lot easier than expanding the allowed character limit for the columns via migration).
new_attributes: new_attributes, | ||
&parse_row | ||
) | ||
ensure |
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.
TIL! :)
Production import:
|
Updates to import 2018-2019 schools. This PR includes:
state_school_id
, which (without these changes) is blocked by the foreign key relationship tostate_cs_offerings
. The approach here is to destroy (delete) the existing state CS offerings temporarily, update the school'sstate_school_id
, then recreate thestate_cs_offerings
.To do (specific to this import):
(other) to do in NCES import space:
school_stats_by_year
.Testing story
I've included tests to cover the dry run import of a CSV, and the new reconstitution of deleted state CS offerings.
I also tested the import manually, by seeding my schools table from scratch (without the new block for 2018-2019 data), then after adding the block running `School.seed_from_s3(stub_school_data: false)
Result:
Reviewer Checklist: